[PATCH] D55143: [PowerPC][NFC] Set isPseudo in base class PostRAPseudo

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrFormats.td:2165
 
+class PostRAPseudo<dag OOL, dag IOL, string asmstr, list<dag> pattern>
+    : Pseudo<OOL, IOL, asmstr, pattern> {
----------------
jsji wrote:
> steven.zhang wrote:
> > steven.zhang wrote:
> > > The logic here is quite weird. Maybe, we could set the isPseudo inside the Pseudo and use the Pseudo as the base class instead of creating a new one. For those places that didn't want the isPseudo to be set, just add a let to clear it. (i.e. when usesCustomInserter is used )
> > It doesn't make sense to me to not to set the isPseudo for the class Pseudo.
> > ```
> > //===----------------------------------------------------------------------===//
> > class Pseudo<dag OOL, dag IOL, string asmstr, list<dag> pattern>
> >     : I<0, OOL, IOL, asmstr, NoItinerary> {
> >   let isCodeGenOnly = 1;
> >   let PPC64 = 0;
> >   let Pattern = pattern;
> >   let Inst{31-0} = 0;
> >   let hasNoSchedulingInfo = 1;
> > }
> > ```
> As I mentioned in description, there are several "Pseudo", isPseudo flag is only for Post RA Pseudo. While Pseudo<>class are for Code Emitter, so setting isPseudo on Pseudo will causing issues. I don't see any advantage of setting it in base class, then unset it...
> 
> Adding PostRAPseudo class is to indicate the difference of Pseudo and PostRAPseudo.
> one more comments about "isPseudo is set for PoseRA Pseudo only" may help?
I agree with the intention of the fix. But has some concern on the name PostRAPseudo, which is just set the isPseudo.  This is the definition of this flag in Target.td
```bit isPseudo     = 0;     // Is this instruction a pseudo-instruction?
                           // If so, won’t have encoding information for
                           // the [MC]CodeEmitter stuff.
```
It is not relative with PreRA or PostRA. And yes, we use this flag to determine if expand the Pseudo instruction or not. However, it is also used by other place to fold imm etc. 

Maybe, we need to distinguish the Pseudo as having the encoding information in CodeEmitter or not.  Such as, PsuedoWithoutEncodingInfo, instead of PostRAPseudo.

Unfortunately, the use of this flag varies among platforms. ARM only set it in the base for Pseudo(as what I suggested before), while AArch64 didn't set it in the base, but set it when define the instructions(what we did now). 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55143