[PATCH] D92449: [X86] Sink x86_amx load in AMX type lowering.

Wed Dec 2 22:05:29 PST 2020

craig.topper added inline comments.

================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:100
+       make_range(Start->getIterator(), End->getIterator())) {
+    if (Inst.mayThrow())
+      return &Inst;
----------------
pengfei wrote:
> LuoYuanke wrote:
> > LuoYuanke wrote:
> > > craig.topper wrote:
> > > > craig.topper wrote:
> > > > > Does this prevent sinking across a call that may change the memory being loaded?
> > > > Or atomics, or anything with side effects.
> > > Thank you for review. Yes. I want to prevent all the scenario that may change the memory being loaded. I'll  check how many scenario we need to prevent from sinking.
> > @craig.topper , I do some study, but I still don't understand why load can't sink across atomic instruction or side effect instruction. I notice in MergedLoadStoreMotion::isStoreSinkBarrierInRange(), it only check myThrow() and alias.
> I only noticed we chain the memory operations during building the DAG and these side effect instructions. Do we have such rules for middle end passes?
Doesn't isStoreSinkBarrierInRange call canInstructionRangeModRef not just isnoalias with a range including every instruction between the store and where it wants to be moved to?

"Release" ordering for an atomic means that no load/store can be moved below it, for example.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92449