Invalid base register with fast-isel

[Hal Finkel]

----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>
> Sent: Friday, June 27, 2014 5:56:40 AM
> Subject: Re: Invalid base register with fast-isel
> 
> Hal Finkel <hfinkel at anl.gov> wrote on 24.06.2014 00:59:06:
> 
> > > From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> > > To: "Hal Finkel" <hfinkel at anl.gov>
> > > Cc: "llvm-commits" <llvm-commits at cs.uiuc.edu>
> > > Sent: Monday, June 23, 2014 7:57:22 AM
> > > Subject: Invalid base register with fast-isel
> 
> > > For some reason, the underlying vreg was allocated as G8RC
> > > instead of
> > > G8RC_NOX0.   That vreg came originally from
> > > PPCRegisterInfo::materializeFrameBaseRegister.  Changing the
> > > register
> > > class there (along the lines of the attached patch) seems to fix
> > > the
> > > problem for me, but I'm not really sure whether this is right
> > > solution.
> > > How is register class selection supposed to work with fast-isel?
> > > Isn't the presence of the vreg inside an address supposed to be
> > > enough
> > > to ensure use of the G8RC_NOX0 class?
> >
> > Yes, it is. But I suspect that the FastISel code that is creating
> > those STW instructions is not correctly setting (or constraining)
> > the register class for the base address operand. Maybe a call to
> > constrainRegClass is needed in PPCFastISel::PPCComputeAddress near
> > where the Instruction::Alloca case is handled?
> 
> I'm not sure I understand exactly what you're suggesting here, sorry
> ...
> 
> In this particular case, FastISel doesn't create the base register
> at all; it simply creates a FrameIndex reference, which is resolved
> in the "Local Stack Slot Allocation" pass (that would presumably
> also handle the SelectionDAG case).
> 
> Before Local Stack Slot Allocation, we have:
> 
>         %vreg183<def> = LI 0; GPRC:%vreg183
>         STW %vreg183, 0, <fi#0>; mem:ST4[%i] GPRC:%vreg183
>         STW %vreg183, 0, <fi#1>; mem:ST4[%j] GPRC:%vreg183
> 
> (The only register involved is %vreg183, which is perfectly fine
> as GPRC; the address is a plain <fi> with no explicit register
> at all at this stage.)
> 
> *After* Local Stack Slot Allocation, we have:
> 
> BB#0: derived from LLVM BB %entry
>         %vreg383<def> = ADDI8 <fi#4>, 0; G8RC:%vreg383
> [... many instructions later ...]
>         %vreg183<def> = LI 0; GPRC:%vreg183
>         STW %vreg183, 10348, %vreg383; mem:ST4[%i] GPRC:%vreg183
> G8RC:%vreg383
>         STW %vreg183, 10344, %vreg383; mem:ST4[%j] GPRC:%vreg183
> G8RC:%vreg383
> 
> At this point the <fi> references have been resolved in terms of an
> intermediate base register, which was introduced (as originally
> mentioned)
> in PPCRegisterInfo::materializeFrameBaseRegister.
> 
> As far as I can see, that latter process took place long after
> FastISel,
> and
> with FastISel having no oppportunity to influence its choices ...
> 
> Am I missing something here?

You're right; but logically, the restriction does not belong on the output from materializeFrameBaseRegister. Can you constrain the register class on the store operand near the end of PPCRegisterInfo::eliminateFrameIndex?

Thanks again,
Hal

> 
> Bye,
> Ulrich
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory