llvm dev - Apr 2015 - [LLVMdev] [RFC][CodeGen] What CLI should we provide for overriding the target decision on whether to run a pass (GlobalMerge) ?

Hi Eric, all,

Currently, there isn't a good way to force enable/disable GlobalMerge.

Targets decide whether to create the pass based on the optimization
level (which is how it should be).

The problem is when you want to override that decision.  We have
-enable-global-merge, true by default, which should really be
-disable-global-merge, as it only works as a last-resort way to
force-disable it (i.e., if the target decided it won't add the pass,
-enable-global-merge does nothing).

I had some patches for LTO up, but let's forget about that and just
worry about command-line options for now.

I see a few solutions, ordered by decreasing ugliness:
- move the CodeGenOpt check to the pass rather than each target
- have targets tell the pass whether they want it enabled by default
(a bool ctor parameter), so the one cl::opt flag can properly override
that
- add -arm-/-aarch64-global-merge flags, that override the CodeGenOpt
default;  teach clang to generate those from -m[no-]global-merge
- do exactly the same thing as BranchFolding/-enable-tail-merge: make
the pass a default part of the codegen pipeline, add a flag to
TargetPassConfig, have targets set the default

I really like the last one, and I haven't seen any FIXME or anything
suggesting it's problematic w.r.t. long-term plans for
LTO/per-function codegen.  The target then looks at the flag, and at
the PassConfig gotten from the MachineFunction, and decides whether to
run.

Thoughts?

-Ahmed

And of course last option isn't possible, because (for now) we need to
pass the maximum offset to createGlobalMergePass.  This isn't a
fundamental limitation though (it used to be given through TLI, with
D8070 and some callbacks we can fix that), so I'm curious about
peoples opinions, if any.

Looking at this again, the other options are really pretty terrible,
so the only realistic choice is the target-specific flags.
I expect this to change with the proper LTO support. The last
iteration of D7968 was a proposal for adding target-specific
attributes to globals.

Anyway, sorry for the noise, there wasn't much room for comments after
all.  If you can think of a better solution I'd love to hear it!

-Ahmed

On Fri, Apr 10, 2015 at 11:54 AM, Ahmed Bougacha
<ahmed.bougacha at gmail.com> wrote:
> Hi Eric, all,
>
> Currently, there isn't a good way to force enable/disable GlobalMerge.
>
> Targets decide whether to create the pass based on the optimization
> level (which is how it should be).
>
> The problem is when you want to override that decision.  We have
> -enable-global-merge, true by default, which should really be
> -disable-global-merge, as it only works as a last-resort way to
> force-disable it (i.e., if the target decided it won't add the pass,
> -enable-global-merge does nothing).
>
> I had some patches for LTO up, but let's forget about that and just
> worry about command-line options for now.
>
> I see a few solutions, ordered by decreasing ugliness:
> - move the CodeGenOpt check to the pass rather than each target
> - have targets tell the pass whether they want it enabled by default
> (a bool ctor parameter), so the one cl::opt flag can properly override
> that
> - add -arm-/-aarch64-global-merge flags, that override the CodeGenOpt
> default;  teach clang to generate those from -m[no-]global-merge
> - do exactly the same thing as BranchFolding/-enable-tail-merge: make
> the pass a default part of the codegen pipeline, add a flag to
> TargetPassConfig, have targets set the default
>
> I really like the last one, and I haven't seen any FIXME or anything
> suggesting it's problematic w.r.t. long-term plans for
> LTO/per-function codegen.  The target then looks at the flag, and at
> the PassConfig gotten from the MachineFunction, and decides whether to
> run.
>
> Thoughts?
>
> -Ahmed

I replied to the current patch you committed, but the only idea I have
otherwise is to make this programmatically possible via either a struct
that's passed down to the TargetMachine creation (something off of
TargetOptions ala the abi bits I put there, but target specific), or Akira
had an idea at one point of passing the options via strings to the backend.
I'm less fond of this, but it is, admittedly, more flexible when versions
differ.

Thoughts?

-eric

On Sat, Apr 11, 2015 at 12:48 AM Ahmed Bougacha <ahmed.bougacha at
gmail.com>
wrote:
> And of course last option isn't possible, because (for now) we need to
> pass the maximum offset to createGlobalMergePass.  This isn't a
> fundamental limitation though (it used to be given through TLI, with
> D8070 and some callbacks we can fix that), so I'm curious about
> peoples opinions, if any.
>
> Looking at this again, the other options are really pretty terrible,
> so the only realistic choice is the target-specific flags.
> I expect this to change with the proper LTO support. The last
> iteration of D7968 was a proposal for adding target-specific
> attributes to globals.
>
> Anyway, sorry for the noise, there wasn't much room for comments after
> all.  If you can think of a better solution I'd love to hear it!
>
> -Ahmed
>
> On Fri, Apr 10, 2015 at 11:54 AM, Ahmed Bougacha
> <ahmed.bougacha at gmail.com> wrote:
> > Hi Eric, all,
> >
> > Currently, there isn't a good way to force enable/disable
GlobalMerge.
> >
> > Targets decide whether to create the pass based on the optimization
> > level (which is how it should be).
> >
> > The problem is when you want to override that decision.  We have
> > -enable-global-merge, true by default, which should really be
> > -disable-global-merge, as it only works as a last-resort way to
> > force-disable it (i.e., if the target decided it won't add the
pass,
> > -enable-global-merge does nothing).
> >
> > I had some patches for LTO up, but let's forget about that and
just
> > worry about command-line options for now.
> >
> > I see a few solutions, ordered by decreasing ugliness:
> > - move the CodeGenOpt check to the pass rather than each target
> > - have targets tell the pass whether they want it enabled by default
> > (a bool ctor parameter), so the one cl::opt flag can properly override
> > that
> > - add -arm-/-aarch64-global-merge flags, that override the CodeGenOpt
> > default;  teach clang to generate those from -m[no-]global-merge
> > - do exactly the same thing as BranchFolding/-enable-tail-merge: make
> > the pass a default part of the codegen pipeline, add a flag to
> > TargetPassConfig, have targets set the default
> >
> > I really like the last one, and I haven't seen any FIXME or
anything
> > suggesting it's problematic w.r.t. long-term plans for
> > LTO/per-function codegen.  The target then looks at the flag, and at
> > the PassConfig gotten from the MachineFunction, and decides whether to
> > run.
> >
> > Thoughts?
> >
> > -Ahmed
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150414/0bdf03e2/attachment.html>