[PATCH] D66991: [PowerPC] Fix SH field overflow issue

[Yi-Hong Lyu via Phabricator via llvm-commits]

Yi-Hong.Lyu marked 2 inline comments as done.
Yi-Hong.Lyu added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/SH-field-overflow.mir:10
+  ; PowerPC Pre-Emit Peephole converts
+  ;   renamable $r4 = LI 0
+  ;   renamable $r5 = SRW renamable $r3, renamable $r4
----------------
jsji wrote:
> Yi-Hong.Lyu wrote:
> > jsji wrote:
> > > We know what exactly we want to test in MIR, so why don't we reduce this MIR test further ?
> > > eg:  a MIR with following lines should be sufficient for 32 bit, you can add another module for 64 bits, and that should be all.
> > > 
> > > ```
> > > $ cat sh-overflow.mir
> > > ---
> > > name:            special_right_shift32_0
> > > liveins:
> > >   - { reg: '$x3'}
> > >   - { reg: '$x4'}
> > > tracksRegLiveness: true
> > > body:             |
> > >   bb.0.entry:
> > >     liveins: $r3, $r4
> > >   
> > >     renamable $r4 = LI 0
> > >     renamable $r3 = SRW renamable $r3, renamable $r4
> > >     BLR8 implicit $lr8, implicit $rm, implicit $x3
> > > ...
> > > 
> > > ```
> > > 
> > Multiple passes after ppc-mi-peepholes relies on SSA form. Apparently, the test case is not in SSA form so we can't just use it.
> Good point, sorry, I did not check SSA constraint for MIR carefully. 
> My example above is mostly just to demo the idea of how to reduce the case
> 
> How about something like these?
> 
> ```
> $ cat t2.mir
> ---
> name:            special_right_shift32_0
> liveins:
>   - { reg: '$r4'}
>   - { reg: '$r5'}
> tracksRegLiveness: true
> body:             |
>   bb.0.entry:
>     liveins: $r5, $r4
>   
>     renamable $r4 = LI 0
>     renamable $r3 = SRW killed renamable $r5, killed renamable $r4, implicit-def $x3
>     BLR8 implicit $lr8, implicit $rm, implicit killed $x3
> ...
> $ cat t3.mir
> ---
> name:            special_right_shift64_0
> liveins:
>   - { reg: '$r4'}
>   - { reg: '$x5'}
> tracksRegLiveness: true
> body:             |
>   bb.0.entry:
>     liveins: $r4, $x5
>   
>     renamable $r4 = LI 0
>     renamable $x3 = SRD killed renamable $x5, killed renamable $r4
>     BLR8 implicit $lr8, implicit $rm, implicit killed $x3
> ...
> ```
Thanks for your test case. Both of your test cases works. One concern is that they are non-SSA MIR tests but we apply some SSA MIR passes (e.g., Live Variable Analysis) on them. I come up with reduced SSA MIR tests, how do you think?


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

https://reviews.llvm.org/D66991