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

[ChenZheng via Phabricator via llvm-commits]

shchenz marked an inline comment as done.
shchenz 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
----------------
steven.zhang wrote:
> 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. 
Yes, the test point here is we should eliminate instruction `%2:gprc = RLWINM %1:gprc, 27, 5, 10`, so IMO it is ok to check if %2:gprc is stilled defined/used. And `%2:gprc` is got from test case input, so I guess if any change wants to change the input symbolic register, it must be aware that the input symbolic register is used in checking lines.


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

https://reviews.llvm.org/D71885