[X86] Fix encoding for ATOMIC_LOGIC_OP

[n-omer]

Summary

In D140939 the following issue was introduced:

multiclass ATOMIC_LOGIC_OP_RM<bits<8> Opc8, string s> {
  let Defs = [EFLAGS], mayLoad = 1, mayStore = 1, isCodeGenOnly = 1,
      SchedRW = [WriteBitTestSetRegRMW]  in {
    def 16rm : Ii8<Opc8, MRMDestMem, (outs), (ins i16mem:$src1, GR16:$src2),           -------> Type Ii8
                  !strconcat(s, "{w}\t{$src2, $src1|$src1, $src2}"),
                  [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR16:$src2))]>,
               OpSize16, TB, LOCK;
    def 32rm : Ii8<Opc8, MRMDestMem, (outs), (ins i32mem:$src1, GR32:$src2),           -------> Type Ii8
                  !strconcat(s, "{l}\t{$src2, $src1|$src1, $src2}"),
                  [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR32:$src2))]>,
               OpSize32, TB, LOCK;
    def 64rm : RIi8<Opc8, MRMDestMem, (outs), (ins i64mem:$src1, GR64:$src2),         -------> Type RIi8
                   !strconcat(s, "{q}\t{$src2, $src1|$src1, $src2}"),
                   [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR64:$src2))]>,
               TB, LOCK;
  }
}

As can be seen above, the ATOMIC_LOGIC_OP_RM definitions use the Ii8/RIi8 encoding, which makes the code emitted by the MC layer incorrect.

The problem can be observed in the test case I've added by looking at the value of the relocation, at the moment the value is off by 1 because of https://github.com/llvm/llvm-project/blob/34de7da6246cdfa6ff6f3d3c514583cddc0a10ec/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp#L581. After applying my patch the relocation is as expected.

https://github.com/llvm/llvm-project/issues/61384

This patch fixes the issue and adds a regression test.

[RKSimon]

We should probably get this merged into 16.x - please can you raise a bug for tracking?

llvm/lib/Target/X86/X86InstrCompiler.td

899

Copy + paste typo from the ATOMIC_LOGIC_OP above I suppose

llvm/test/CodeGen/X86/atomic-rm-bit-test-64.ll

1663 ↗

(On Diff #504615)

Please can you raise a bug and reference it here to speed up triage

[n-omer]

Negative test: https://reviews.llvm.org/D145933

[RKSimon]

Patch itself LGTM but the summary needs a cleanup - the defm section is irrelevant (and confusing) - just explain that ATOMIC_LOGIC_OP_RM shouldn't be using the *i8 instruction encodings

[RKSimon]

LGTM - cheers

llvm/lib/Target/X86/X86InstrCompiler.td

def 64m : RIi8<0xBA, Form, (outs), (ins i64mem:$src1, i8imm:$src2),
                    [(set EFLAGS, (!cast<SDNode>("x86" # s) addr:$src1, timm:$src2, (i32 64)))]>,
               TB, LOCK;
   }
 }
 
 multiclass ATOMIC_LOGIC_OP_RM<bits<8> Opc8, string s> {
   let Defs = [EFLAGS], mayLoad = 1, mayStore = 1, isCodeGenOnly = 1,
       SchedRW = [WriteBitTestSetRegRMW]  in {
-    def 16rm : Ii8<Opc8, MRMDestMem, (outs), (ins i16mem:$src1, GR16:$src2),
+    def 16rm : I<Opc8, MRMDestMem, (outs), (ins i16mem:$src1, GR16:$src2),
                   !strconcat(s, "{w}\t{$src2, $src1|$src1, $src2}"),
                   [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR16:$src2))]>,
                OpSize16, TB, LOCK;
-    def 32rm : Ii8<Opc8, MRMDestMem, (outs), (ins i32mem:$src1, GR32:$src2),
+    def 32rm : I<Opc8, MRMDestMem, (outs), (ins i32mem:$src1, GR32:$src2),
                   !strconcat(s, "{l}\t{$src2, $src1|$src1, $src2}"),
                   [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR32:$src2))]>,
                OpSize32, TB, LOCK;
-    def 64rm : RIi8<Opc8, MRMDestMem, (outs), (ins i64mem:$src1, GR64:$src2),
+    def 64rm : RI<Opc8, MRMDestMem, (outs), (ins i64mem:$src1, GR64:$src2),
                    !strconcat(s, "{q}\t{$src2, $src1|$src1, $src2}"),
                    [(set EFLAGS, (!cast<SDNode>("x86_rm_" # s) addr:$src1, GR64:$src2))]>,
                TB, LOCK;
   }
 }
 
 
 defm LOCK_BTS : ATOMIC_LOGIC_OP<MRM5m, "bts">;

llvm/test/CodeGen/X86/pr61384.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-unknown-linux-gnu -show-mc-encoding < %s | FileCheck %s
 
 @a = external dso_local global { { i64 } }
 
 define i32 @atomic_global() nounwind {
 ; CHECK-LABEL: atomic_global:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    xorl %eax, %eax # encoding: [0x31,0xc0]
 ; CHECK-NEXT:    lock btsq %rax, a(%rip) # encoding: [0xf0,0x48,0x0f,0xab,0x05,A,A,A,A]
-; CHECK-NEXT:    # fixup A - offset: 5, value: a-5, kind: reloc_riprel_4byte
+; CHECK-NEXT:    # fixup A - offset: 5, value: a-4, kind: reloc_riprel_4byte
 ; CHECK-NEXT:    xorl %eax, %eax # encoding: [0x31,0xc0]
 ; CHECK-NEXT:    retq # encoding: [0xc3]
 entry:
   %shl.i = shl i64 1, 0
   %0 = atomicrmw or ptr @a, i64 %shl.i monotonic, align 8
   %and.i = and i64 %shl.i, %0
   ret i32 0
 }