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

Phoebe Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 5 18:39:43 PST 2023


pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:5653
       Info.ptrVal = I.getArgOperand(0);
       unsigned Size = I.getType()->getScalarSizeInBits();
       Info.memVT = EVT::getIntegerVT(I.getType()->getContext(), Size);
----------------
goldstein.w.n wrote:
> pengfei wrote:
> > Should the `Size` be same with operand 1 rather than result?
> > Should the `Size` be same with operand 1 rather than result?
> 
> If I change it I run into assertion errors.
> 
> `I.getArgOperand(0)->getType()->getScalarSizeInBits()` the `Size` can be zero (bitwidth too small)
> 
> or `I.getArgOperand(1)->getType()->getScalarSizeInBits()` I run into:
> ```
> llc: /home/noah/programs/opensource/llvm-dev/src/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:11108: llvm::MemSDNode::MemSDNode(unsigned int, unsigned int, const llvm::DebugLoc&, llvm::SDVTList, llvm::EVT, llvm::MachineMemOperand*): Assertion `memvt.getStoreSize().getKnownMinSize() <= MMO->getSize() && "Size mismatch!"' failed.
> ```
> 
> Maybe that means there is a bug in the patch elsewhere?
> 
> Running the test here are the sizes I see:
> ```
> Sizes: 8 / 0 / 16
> Sizes: 8 / 0 / 16
> Sizes: 8 / 0 / 16
> Sizes: 8 / 0 / 16
> Sizes: 16 / 0 / 8
> Sizes: 16 / 0 / 8
> Sizes: 16 / 0 / 8
> Sizes: 16 / 0 / 8
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 8 / 0 / 32
> Sizes: 32 / 0 / 8
> Sizes: 32 / 0 / 8
> Sizes: 32 / 0 / 8
> Sizes: 32 / 0 / 8
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 8 / 0 / 64
> Sizes: 64 / 0 / 8
> Sizes: 64 / 0 / 8
> Sizes: 64 / 0 / 8
> ```
I mean split `x86_atomic_bt*_rm` from it and then change it to `I.getArgOperand(1)->getType()->getScalarSizeInBits()`
Other intrinsics are correct to use `I.getType()->getScalarSizeInBits()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140939



More information about the llvm-commits mailing list