llvm dev - Nov 2015 - Mips unconditionally uses fast-isel?

> > > > I was mucking around in FastISel, and was surprised to see
the test
> > > >   llvm/test/CodeGen/Mips/emergency-spill-slot-near-fp.ll
> > > > failed.  This was surprising because it specifies
-fast-isel=false.
> > > >
> > > > Does the Mips code generator use fast-isel even when you ask
it not
> to?
> > > > Thanks,
> > > > --paulr
> > >
> > > This seems to be an all-targets bug. There's code in
OptLevelChanger
> > > (lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp) that forces Fast
ISel
> to
> > > be enabled for every -O0 function.
> >
> > Well.... the test specifies -O0 so even though the function is marked
> > 'optnone' I'd expect OptLevelChanger to be a no-op,
because it checks
> > whether the old and new OptLevel are the same. Which they should be.
> >
> > If I use -debug-only=isel I see it is changing from -O2 to -O0, which
> > makes me think the "real" bug is that llc is ignoring -O0.
> > --paulr
> 
> Looking further, I think it's a bit of both. Enabling Fast ISel when -
> fast-isel=false is given doesn't make sense but there are also five
> targets (Mips, BPF, Sparc, PPC, and AMDGPU) that use the
> SelectionDAGISel(TargetMachine &) constructor which leave the
optimization
> level at the default. The other targets all pass on the optimization level
> using the two argument constructor.
Okay... so Mips unconditionally has -O2 behavior from SelectionDAGISel,
regardless of the requested optimization level; is that really what you 
wanted?
												|
For this particular test, the -O0 is ignored and becomes -O2, which 
defaults to -fast-isel=false, and then 'optnone' is actually where you 
get the -O0 effect. Except that 'optnone' also turns ON fast-isel, which
you didn't notice because it didn't actually matter, until I came along 
and started mucking around in FastISel.

I think if Mips wants -O0 to work in llc, then it really needs to be 
passed in to SelectionDAGISel.  Then take 'optnone' off the function in 
this test, and it should all Just Work.  And the test would actually be
exercising the non-fast-isel path, which it seems is what you wanted?

The other thing that might work, is having TargetMachine remember how
the fast-isel option got set, and make OptLevelChanger do the right
thing. But that seems like a hack to work around Mips not obeying the
specified optimization level, honestly.

Thanks,
--paulr

> Okay... so Mips unconditionally has -O2 behavior from SelectionDAGISel,
> regardless of the requested optimization level; is that really what you
> wanted?
This code slightly pre-dates my joining LLVM so I don't know the thoughts
behind it. I agree it looks wrong though.
> For this particular test, the -O0 is ignored and becomes -O2, which
> defaults to -fast-isel=false, and then 'optnone' is actually where
you
> get the -O0 effect. Except that 'optnone' also turns ON fast-isel,
which
> you didn't notice because it didn't actually matter, until I came
along
> and started mucking around in FastISel.
I think that sums it up. One observation to add: If I make
SelectionDAGISel::runOnMachineFunction() unconditionally print the effective
optimization level. Mips always picks -O2 as we've discussed but I can't
get -O1 for X86 either (I get -O2 instead).
> I think if Mips wants -O0 to work in llc, then it really needs to be
> passed in to SelectionDAGISel.  Then take 'optnone' off the
function in
> this test, and it should all Just Work.  And the test would actually be
> exercising the non-fast-isel path, which it seems is what you wanted?
I agree.
> The other thing that might work, is having TargetMachine remember how
> the fast-isel option got set, and make OptLevelChanger do the right
> thing. But that seems like a hack to work around Mips not obeying the
> specified optimization level, honestly.
I think we should do that as well. I don't think it's right that optnone
enables Fast ISel even when it's been explicitly disabled. It should do the
same checks as addPassesToGenerateCode() does.

> > The other thing that might work, is having TargetMachine remember how
> > the fast-isel option got set, and make OptLevelChanger do the right
> > thing. But that seems like a hack to work around Mips not obeying the
> > specified optimization level, honestly.
> 
> I think we should do that as well. I don't think it's right that
optnone
> enables Fast ISel even when it's been explicitly disabled. It should do
> the same checks as addPassesToGenerateCode() does.
Hm?  What you're asking for is that "-O2" and "-O2
-fast-isel=none" are
identical, unless you have an 'optnone' function. Do you really have a
use-case for controlling the codegen path for an 'optnone' function?
The whole point of 'optnone' is to avoid optimizations.
--paulr