[PATCH] D71885: [PowerPC] replace rlwinm operand 1 with src rlwinm operand 1

qshanz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 30 03:54:48 PST 2019


steven.zhang added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/fold-rlwinm.mir:137
+    ; CHECK-NOT: %2:gprc
     %3:gprc = RLWINMo %2:gprc, 8, 5, 10, implicit-def $cr0
+    ; CHECK: %3:gprc = ANDIo %1, 0, implicit-def $cr0
----------------
shchenz wrote:
> shchenz wrote:
> > steven.zhang wrote:
> > > steven.zhang wrote:
> > > > Why the RLWINMo is still here ?
> > > Ah, sorry. I didn't notice that, you are replacing it with ANDIo. So, it is fine. But I still don't agree with checking the symbol register number which is vulnerable.
> > Here what we check is that original input for MI (%2) is not needed?  I am glad if you can point out why Check-not %2 is not good? 
> If we do something wrongly in future to change 2% to the result of other opcode, we should still can eliminate it. This can only caught by current CHECK-NOT, but can not checked by CHECK-NOT %2  RLWINM?  @steven.zhang? 
Your test point should align with your implementation. If it is change to other opcode in the future, it is hard to say if it is right or wrong as it is not your test point. And besides, the symbolic register is subject to change. 


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

https://reviews.llvm.org/D71885





More information about the llvm-commits mailing list