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

[sheisc]

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}'

[pengfei]

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

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.

[pengfei]

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]

In D125789#3519361, @pengfei wrote:

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 :)

[sheisc]

In D125789#3519411, @pengfei wrote:

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.

[pengfei]

In D125789#3519433, @sheisc wrote:

In D125789#3519411, @pengfei wrote:

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 :)

[sheisc]

In D125789#3519493, @pengfei wrote:

In D125789#3519433, @sheisc wrote:

In D125789#3519411, @pengfei wrote:

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.

[craig.topper]

Is it gcc that needs a fix or binutils?

[sheisc]

In D125789#3519538, @craig.topper wrote:

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/

clang/docs/ReleaseNotes.rst

- Improved ``-O0`` code generation for calls to ``std::move``, ``std::forward``,
   are now treated as compiler builtins and implemented directly, rather than
   instantiating the definition from the standard library.
 - Fixed mangling of nested dependent names such as ``T::a::b``, where ``T`` is a
   template parameter, to conform to the Itanium C++ ABI and be compatible with
   GCC. This breaks binary compatibility with code compiled with earlier versions
   of clang; use the ``-fclang-abi-compat=14`` option to get the old mangling.
 - Preprocessor character literals with a ``u8`` prefix are now correctly treated as
   unsigned character literals. This fixes `Issue 54886 <https://github.com/llvm/llvm-project/issues/54886>`_.
-- Stopped allowing constriants on non-template functions to be compliant with
+- Stopped allowing constraints on non-template functions to be compliant with
   dcl.decl.general p4.
 
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 - Diagnose consteval and constexpr issues that happen at namespace scope. This
   partially addresses `Issue 51593 <https://github.com/llvm/llvm-project/issues/51593>`_.
 - No longer attempt to evaluate a consteval UDL function call at runtime when
   it is called through a template instantiation. This fixes

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

 //===-- X86InstComments.cpp - Generate verbose-asm comments for instrs ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static void printMasking(raw_ostream &OS, const MCInst *MI,
   if (Desc.getOperandConstraint(MaskOp, MCOI::TIED_TO) != -1)
     ++MaskOp;
 
   const char *MaskRegName = getRegName(MI->getOperand(MaskOp).getReg());
 
   // MASK: zmmX {%kY}
   OS << " {%" << MaskRegName << "}";
 
   // MASKZ: zmmX {%kY} {z}
   if (MaskWithZero)
-    OS << " {z}";
+    OS << "{z}";
 }
 
 static bool printFMAComments(const MCInst *MI, raw_ostream &OS,
                              const MCInstrInfo &MCII) {
   const char *Mul1Name = nullptr, *Mul2Name = nullptr, *AccName = nullptr;
   unsigned NumOperands = MI->getNumOperands();
   bool RegForm = false;
   bool Negate = false;

llvm/lib/Target/X86/X86MCInstLower.cpp

 //===-- X86MCInstLower.cpp - Convert X86 MachineInstr to an MCInst --------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
static std::string getShuffleComment(const MachineInstr *MI, unsigned SrcOp1Idx,
   if (SrcOp1Idx > 1) {
     assert((SrcOp1Idx == 2 || SrcOp1Idx == 3) && "Unexpected writemask");
 
     const MachineOperand &WriteMaskOp = MI->getOperand(SrcOp1Idx - 1);
     if (WriteMaskOp.isReg()) {
       CS << " {%" << GetRegisterName(WriteMaskOp.getReg()) << "}";
 
       if (SrcOp1Idx == 2) {
-        CS << " {z}";
+        CS << "{z}";
       }
     }
   }
 
   CS << " = ";
 
   for (int i = 0, e = ShuffleMask.size(); i != e; ++i) {
     if (i != 0)