This is an archive of the discontinued LLVM Phabricator instance.

FIX the assembly format of the x86 backend to make both clang and gcc happy
AbandonedPublic

Authored by sheisc on May 17 2022, 7:23 AM.

Details

Reviewers
None
Summary

The GNU assembler (used in gcc) complains about the syntactic correctness of
the assembly code (at line 9) generated by clang (with the option -no-integrated-as).

We need to delete the white space in "{%k1} {z}" (at line 9) to make both GCC and LLVM happy.

iron@CSE:demo$ cat -n main.s

 1		.text
 2		.file	"main.c"
 3		.globl	main                    # -- Begin function main
 4		.p2align	4, 0x90
 5		.type	main,@function
 6	main:                                   # @main
 7		.cfi_startproc
 8	# %bb.0:                                # %entry
 9		vmovdqu16 %zmm5 , %zmm5 {%k1} {z}
10		xorl	%eax, %eax
11		retq
12	.Lfunc_end0:
13		.size	main, .Lfunc_end0-main
14		.cfi_endproc
15	                                        # -- End function
16
17		.ident	"clang"
18		.section	".note.GNU-stack","",@progbits

iron@CSE:demo$ clang -c main.s -o main.o

iron@CSE:demo$ gcc -c main.s -o main.o

main.s: Assembler messages:
main.s:9: Error: unknown vector operation: ` {z}'

Diff Detail

Event Timeline

sheisc created this revision.May 17 2022, 7:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2022, 7:23 AM
sheisc requested review of this revision.May 17 2022, 7:23 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 17 2022, 7:23 AM

I guess a lot of lines of tests need to update

$ grep -rn " {z}" llvm/test/CodeGen/X86/ | wc -l
7797
clang/docs/ReleaseNotes.rst
368 ↗(On Diff #430052)

I think this can be a sepreate patch. Or commit directly with a NFC label.

llvm/lib/Target/X86/MCTargetDesc/X86InstComments.cpp
279

Fix the comment as well.

sheisc updated this revision to Diff 430067.May 17 2022, 8:15 AM
sheisc retitled this revision from Fix release note typo from 6da3d66f to FIX the assembly format of the x86 backend to make both clang and gcc happy.
sheisc edited the summary of this revision. (Show Details)
sheisc added a project: Restricted Project.

I think another way is to report the issue to GCC. From the perspective of the user, GCC should support both {%k1} {z} and {%k1}{z}. Then we don't need the clange on LLVM.

sheisc abandoned this revision.May 17 2022, 8:21 AM

I guess a lot of lines of tests need to update

$ grep -rn " {z}" llvm/test/CodeGen/X86/ | wc -l
7797

Thanks.
Yes. It seems to be a big revision, caused by only one white space :)

I think another way is to report the issue to GCC. From the perspective of the user, GCC should support both {%k1} {z} and {%k1}{z}. Then we don't need the clange on LLVM.

Yes. It is a good idea.
However, it appears that there is no such white space in the instructions as described in Intel's manuals.
So I don't know which one should be the correct format.
Anyway, not a big issue.
I found this problem when using the fuzzer (i.e. AFL) to build Firefox.

I think another way is to report the issue to GCC. From the perspective of the user, GCC should support both {%k1} {z} and {%k1}{z}. Then we don't need the clange on LLVM.

Yes. It is a good idea.
However, it appears that there is no such white space in the instructions as described in Intel's manuals.
So I don't know which one should be the correct format.
Anyway, not a big issue.
I found this problem when using the fuzzer (i.e. AFL) to build Firefox.

Yeah. This is an interesting question. I didn't notice the difference between LLVM and GCC. I think either way changing here or GCC is OK :)

I think another way is to report the issue to GCC. From the perspective of the user, GCC should support both {%k1} {z} and {%k1}{z}. Then we don't need the clange on LLVM.

Yes. It is a good idea.
However, it appears that there is no such white space in the instructions as described in Intel's manuals.
So I don't know which one should be the correct format.
Anyway, not a big issue.
I found this problem when using the fuzzer (i.e. AFL) to build Firefox.

Yeah. This is an interesting question. I didn't notice the difference between LLVM and GCC. I think either way changing here or GCC is OK :)

Hi Pengfei,

Thanks a lot. The guys in the GCC community can make our life easier by supporting the white space.

Is it gcc that needs a fix or binutils?

Is it gcc that needs a fix or binutils?

Yes. Exactly speaking, it is the GNU assembler in binutils.
https://www.gnu.org/software/binutils/