Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/asm: Token pasting in assembler macros no longer works from go 1.5 onwards #13499

Closed
ncw opened this issue Dec 5, 2015 · 2 comments
Closed

cmd/asm: Token pasting in assembler macros no longer works from go 1.5 onwards #13499

ncw opened this issue Dec 5, 2015 · 2 comments
Labels
FrozenDueToAge
Milestone
Unplanned

Comments

@ncw
Copy link
Contributor

ncw commented Dec 5, 2015

Pre go 1.5 it was possible to use K&R style token pasting in assembler macros with /**/.

Here is an example from the iprime project. In order to avoid passing lots of labels to the MOD_REDUCE macro, token pasting is used to synthesize the labels for the inner macros.

// Compute x = x - y mod p
// Preserves y, p
#define MOD_SUB(x, y, p, LABEL) \
        SUBQ    y, x; \
        JCC     LABEL; \
        ADDQ    p, x; \
LABEL:  \

// Compute x = x + y mod p, using t
// Preserves y, p
#define MOD_ADD(x, y, t, p, LABEL) \
        MOVQ    p, t; \
        SUBQ    y, t; \
        MOD_SUB(x, t, p, LABEL); \

// Reduce 128 bits mod p (b, a) -> a
// Using t0, t1
// This is much faster (2 or 3 times) than DIVQ
#define MOD_REDUCE(b, a, t0, t1, p, label) \
        MOVL    b, t0;  /* Also sets upper 32 bits to 0 */ \
        SHRQ    $32,b ; \
 ; \
        CMPQ    a,p ; \
        JCS     label/**/1 ; \
        SUBQ    p,a ; \
label/**/1: ; \
 ; \
        MOVLQZX t0,t1 ; \
        MOD_SUB(a, t1, p, label/**/2) ; \
 ; \
        MOVLQZX b,t1 ; \
        MOD_SUB(a, t1, p, label/**/3) ; \
 ; \
        SHLQ    $32,t0 ; \
        MOD_ADD(a, t0, t1, p, label/**/4) ; \

(Token pasting is also very useful in ARM assembler when you can make a whole instruction sequence conditional.)

Pre go 1.5 the above worked just fine, but post go 1.5 it doesn't because (I conjecture) the comments /**/ are being replaced with ' ' rather than ''.
@ncw ncw mentioned this issue Dec 5, 2015
Closed
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Dec 5, 2015
@robpike
Copy link
Contributor

robpike commented Dec 18, 2015

I do not plan to fix this. Not exactly working as intended, but not worth fixing.

The design of the assembler, using the go/scanner package as the low-level lexer, makes this annoying to fix. The assembler doesn't see spaces, it sees a stream of tokens (and a comment is seen as a token). I suppose it might be possible to observe that a comment fills the precise gap between the token before and the token after, and that within a macro definition, that implies token pasting, but that is a very fiddly and particular detail I'd prefer not to inject (assuming it's even possible).

This is a narrow, rare, and peculiar compatibility detail that, in the example here at least, is easy to work around using numbered offset labels (3(PC) etc.). I do not think it's worth fixing.

The existence of a C preprocessor in the assembler is mostly a historical artifact that is occasionally useful but can encourage trickery. For the long-term simplicity of the assembler and its input, I'd rather discourage trickery.

I also observe that the preprocessor used here, even in the old assembler, is pre-ANSI C in design. If you want a modern preprocessor of any form, I suggest using one and coupling it with go generate.

@robpike robpike closed this as completed Dec 18, 2015
@ncw
Copy link
Contributor Author

ncw commented Dec 18, 2015

I'm happy with that resolution - less macro abuse is probably better for everyone!

@golang golang locked and limited conversation to collaborators Dec 29, 2016
@rsc rsc unassigned robpike Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@ncw @ianlancetaylor @robpike @gopherbot