[PATCH] D120199: [X86] Use bit test instructions to optimize some logic atomic operations

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:5450
+      Info.memVT = EVT::getIntegerVT(I.getType()->getContext(), Size);
+      Info.align = Align(Size);
+      Info.flags |= MachineMemOperand::MOLoad | MachineMemOperand::MOStore;
----------------
craig.topper wrote:
> Does something check natural alignment before we create the intrinsic?
Yes, it's checked by `atomicSizeSupported` in AtomicExpandPass.cpp


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:5451
+      Info.align = Align(Size);
+      Info.flags |= MachineMemOperand::MOLoad | MachineMemOperand::MOStore;
+      return true;
----------------
craig.topper wrote:
> Does this need MOVolatile?
Maybe not. `MOVolatile` is usually set when `isVolatile()` return true in load and store instructions. We don't have such info in a target intrinsic.
But seems we never worried about it, and there're few place it is set in X86 code. So I guess X86 is fine with or without `MOVolatile`?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:30515
+  Value *Addr =
+      Builder.CreateBitCast(AI->getPointerOperand(), Type::getInt8PtrTy(Ctx));
+  Value *Result = Builder.CreateCall(
----------------
craig.topper wrote:
> craig.topper wrote:
> > CreateBitCast -> createPointerCast
> Probably doesn't really matter, but you should pass the address space to getInt8PtrTy.
I didn't see we pass address space when using `CreatePointerCast`, I guess the address space keep unchanged?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120199