[PATCH] D33519: [PPC] Lower llvm.ppc.cfence(constant) to no-op.

[Tim Shen via Phabricator via llvm-commits]

timshen marked an inline comment as done.
timshen added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8220
+      return Op.getOperand(0);
+    }
     assert(Subtarget.isPPC64() && "Only 64-bit is supported for now.");
----------------
kbarton wrote:
> I agree with this, but I'm wondering if it could/should be generalized a bit more. 
> Are there other cases of SDNodes that we could reach here that are not valid? 
> Alternatively, is it possible to enumerate the legal SD nodes that could be here and remove everything that is not legal? 
I dag a bit more and found the root cause, which is fixed in D33573. This patch, however only makes things faster, not more correct.

The optimization opportunity comes from that, a pass proves at compile time, that the load instruction itself doesn't synchronize with any stores, therefore it's Ok to change the parameter to @llvm.ppc.cfence to something else.

The interesting bits are "at compile time". This basically limits the optimization to ConstantSDNode only - other nodes represent behaviors at runtime, and are unlikely to be proven not synchronized with any stores whatsoever.

I suggest to keep these two cases (constant and others), until we find other pessimizations. Does it make sense?


https://reviews.llvm.org/D33519