[PATCH] D140939: [X86] Transform AtomicRMW logic operations to BT{R|C|S} if only changing/testing a single bit.

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

In D140939#4067362 <https://reviews.llvm.org/D140939#4067362>, @mysterymath wrote:

> Hey, it looks like this change triggered an assertion failure in the Fuchsia continous build: https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-release/b8791734713064712625/overview
>
>   clang: llvm/lib/Target/X86/X86ISelLowering.cpp:31525: TargetLowering::AtomicExpansionKind llvm::X86TargetLowering::shouldExpandLogicAtomicRMWInIR(AtomicRMWInst *) const: Assertion `I->getOperand(0)...
>   PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
>   Stack dump:
>   0.	Program arguments: ../../../recipe_cleanup/clangs6i4kjx7/bin/clang -MD -MF obj/src/connectivity/wlan/drivers/third_party/intel/iwlwifi/mvm/mvm.sta.c.o.d -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS ...
>   1.	<eof> parser at end of file
>   2.	Code generation
>   3.	Running pass 'Function Pass Manager' on module '../../src/connectivity/wlan/drivers/third_party/intel/iw...

The easiest thing to do now would be:

  -  assert(I->getOperand(0) == AI);
  +  if (I->getOperand(0) != AI)
  +    return AtomicExpansionKind::CmpXChg;

or revert.

Although I think if that assertion is being hit the old code was buggy:

  // The following instruction must be a AND single bit.
  auto *C2 = dyn_cast<ConstantInt>(I->getOperand(1));
  unsigned Bits = AI->getType()->getPrimitiveSizeInBits();
  if (!C2 || Bits == 8 || !isPowerOf2_64(C2->getZExtValue()))
    return AtomicExpansionKind::CmpXChg;
  
  if (AI->getOperation() == AtomicRMWInst::And)
    return ~C1->getValue() == C2->getValue()
               ? AtomicExpansionKind::BitTestIntrinsic
               : AtomicExpansionKind::CmpXChg;
  
  return C1 == C2 ? AtomicExpansionKind::BitTestIntrinsic
                  : AtomicExpansionKind::CmpXChg;

Was assuming the above assert AFAICT.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140939/new/

https://reviews.llvm.org/D140939