[X86] Use inlineasm flag output for the _bittest* intrinsics.

[craig.topper]

Summary

Instead of expliciting emitting a setc in the inline asm instructions,
we can use flag output. This allows the backend to use the flag
directly if it is needed by a branch. Previously we needed a test
instruction to convert the register back to a flag.

If the flag can't be used directly, the backend will emit a setcc.

[RKSimon]

Do we have sufficient backend test coverage for this?

[craig.topper]

In D87888#2281360, @RKSimon wrote:

Do we have sufficient backend test coverage for this?

Taking a closer look, I don't think our coverage is very good. We just have one test for each the 28 condition code strings we support and they are all identical tests that just change the string. So we don't have tests that use the condition code as a branch condition. But the code just emits a EFLAGS copy and X86ISD::SETCC so existing combines probably just work.

I did fine a copy paste mistake on one of the condition strings while reviewing the asm flags code. Fixed in 58ecbbcdcddf80b289c449b86d5996d58fba0d78

[craig.topper]

@rnk should this inline asm have a memory clobber on it?

[rnk]

Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.

[craig.topper]

In D87888#2285878, @rnk wrote:

Honestly, I forget exactly what the memory clobber does beyond the "sideeffect" marker. I would expect LLVM to model these just as external function calls that could read or write memory that is passed to them.

The middle end may not care about the memory clobber. I think in the backend the memory clobber will cause mayLoad and mayStore to be set on the INLINASM instruction. Not sure if that does anything that sideeffects didn't already do.

[craig.topper]

Rebase and add a backend test file

[RKSimon]

LGTM @rnk ?

[rnk]

Yep, neat.

clang/lib/CodeGen/CGBuiltin.cpp

 //===---- CGBuiltin.cpp - Emit LLVM Code for builtins ---------------------===//
 //
 // 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 llvm::Value *EmitX86BitTestIntrinsic(CodeGenFunction &CGF,
   // Build the assembly.
   SmallString<64> Asm;
   raw_svector_ostream AsmOS(Asm);
   if (BT.Interlocking != BitTest::Unlocked)
     AsmOS << "lock ";
   AsmOS << "bt";
   if (Action)
     AsmOS << Action;
-  AsmOS << SizeSuffix << " $2, ($1)\n\tsetc ${0:b}";
+  AsmOS << SizeSuffix << " $2, ($1)";
 
   // Build the constraints. FIXME: We should support immediates when possible.
-  std::string Constraints = "=r,r,r,~{cc},~{flags},~{fpsr}";
+  std::string Constraints = "={@ccc},r,r,~{cc},~{flags},~{fpsr}";
   llvm::IntegerType *IntType = llvm::IntegerType::get(
       CGF.getLLVMContext(),
       CGF.getContext().getTypeSize(E->getArg(1)->getType()));
   llvm::Type *IntPtrType = IntType->getPointerTo();
   llvm::FunctionType *FTy =
       llvm::FunctionType::get(CGF.Int8Ty, {IntPtrType, IntType}, false);
 
   llvm::InlineAsm *IA =

clang/test/CodeGen/bittest-intrin.c

void test_arm(long *base, long idx) {
   sink = _interlockedbittestandreset_nf(base, idx);
   sink = _interlockedbittestandset_acq(base, idx);
   sink = _interlockedbittestandset_rel(base, idx);
   sink = _interlockedbittestandset_nf(base, idx);
 }
 #endif
 
 // X64-LABEL: define dso_local void @test32(i32* %base, i32 %idx)
-// X64: call i8 asm sideeffect "btl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
-// X64: call i8 asm sideeffect "btcl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
-// X64: call i8 asm sideeffect "btrl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
-// X64: call i8 asm sideeffect "btsl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
-// X64: call i8 asm sideeffect "lock btrl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
-// X64: call i8 asm sideeffect "lock btsl $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "btl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "btcl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "btrl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "btsl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "lock btrl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
+// X64: call i8 asm sideeffect "lock btsl $2, ($1)", "={@ccc},r,r,~{{.*}}"(i32* %{{.*}}, i32 {{.*}})
 
 // X64-LABEL: define dso_local void @test64(i64* %base, i64 %idx)
-// X64: call i8 asm sideeffect "btq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
-// X64: call i8 asm sideeffect "btcq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
-// X64: call i8 asm sideeffect "btrq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
-// X64: call i8 asm sideeffect "btsq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
-// X64: call i8 asm sideeffect "lock btrq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
-// X64: call i8 asm sideeffect "lock btsq $2, ($1)\0A\09setc ${0:b}", "=r,r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "btq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "btcq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "btrq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "btsq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "lock btrq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
+// X64: call i8 asm sideeffect "lock btsq $2, ($1)", "={@ccc},r,r,~{{.*}}"(i64* %{{.*}}, i64 {{.*}})
 
 // ARM-LABEL: define dso_local {{.*}}void @test32(i32* %base, i32 %idx)
 // ARM: %[[IDXHI:[^ ]*]] = ashr i32 %{{.*}}, 3
 // ARM: %[[BASE:[^ ]*]] = bitcast i32* %{{.*}} to i8*
 // ARM: %[[BYTEADDR:[^ ]*]] = getelementptr inbounds i8, i8* %[[BASE]], i32 %[[IDXHI]]
 // ARM: %[[IDX8:[^ ]*]] = trunc i32 %{{.*}} to i8
 // ARM: %[[IDXLO:[^ ]*]] = and i8 %[[IDX8]], 7
 // ARM: %[[BYTE:[^ ]*]] = load i8, i8* %[[BYTEADDR]], align 1