[llvm-dev] trouble hoisting GlobalValues

[Steve King via llvm-dev]

Hi Guys,

TL;DR: Perhaps we need ISD::OPAQUE instead of more abuse of ISD::BITCAST.

For global values, none of the suggested solutions work well.  The
root of the matter is that we overload ISD::BITCAST with the implicit
role of "ISD::OPAQUE".  However, existing code doesn't treat
ISD::BITCAST as an generally opaque value, since bitcasts and opacity
are two different semantics with sometimes opposite behavior.

Some specific problems:
1) In more than one well buried code path, the compiler
opportunistically removes ISD::BITCAST.  This makes perfect sense for
a BITCAST, but is the opposite of what we want for opaque value.
2) A special exception was coded for a bitcast of a ConstantInt that
prevents premature removal of the BITCAST.  The overall behavior is
non-obvious since it fooled us all into thinking that bitcasts create
more general opaque values.
3) Instruction definition solutions like Zia's work only in the scope
of a basic block (correct?).  This approach also requires targets to
code exceptions for every instruction that *might* use an opaque
value, rather that just one instruction to handle an opaque value.
Long term, my feeling is that this approach requires too much
redundancy.
4) In my specific case, I had a very frustrating time trying to add
global values into the opaque-ifying code paths used by ConstantInt.
Many of the code paths were new to me, but I bumped into subtle
problems trying to sneak global values through in this manner
sometimes due to (1) above.


What do you all think of a first class ISD::OPAQUE operation?  The
basic concept embodied is that optimizations, mostly folding, should
not look through OPAQUE.  ISD::OPAQUE would not imply any casting or
transform.

Some positives:
1) We avoid an ever increasing number of exceptions being added to
BITCAST handling to create opaque values.
2) ISD::OPAQUE could at least be used in a global and constant
hoisting passes, maybe elsewhere.
3) Specical case BITCAST handling for ConstantInt special case moves
to ISD::OPAQUE where it's semantically more accurate.  Target
independent code resolves ISD::OPAQUE as a constant load as we do for
BITCAST today.  Targets never see it.
4) Targets that enable a hypothetical global hoisting would lower
ISD::OPAQUE as an immediate-to-register operation.  No need to
duplicate the rest of their instruction definitions that might touch
an opaque value.

Some negatives:
1) Yet another IR instruction
2) Usage is somewhat niche, suited mostly to minimal code size optimizations.

Does any of this sound sensible?

Regards,
-steve



On Mon, Dec 14, 2015 at 9:55 AM, Ansari, Zia via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> +1
>
> As Hal mentioned, ConstantHoisting.cpp should be able to do this fine for global values. You just need to be careful because I found that the CG needs a little work to make sure that the extra global register pressure doesn't cause any issues.
>
> For local values, if you don't have it, check out my patch : http://reviews.llvm.org/D11363
>
> This will run at ISEL/DAG with the intent of being more precise in the cases that it targets/catches, and to run at a point where more opportunities are exposed. It was worth a fair bit of code size (over 1% on spec2000), and can be extended to catch more cases, if need be.
>
> Thanks,
> Zia.
>
> |-----Original Message-----
> |From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of Hal
> |Finkel via llvm-dev
> |Sent: Friday, December 11, 2015 2:33 PM
> |To: Rafael Espíndola
> |Cc: llvm-dev
> |Subject: Re: [llvm-dev] trouble hoisting GlobalValues
> |
> |----- Original Message -----
> |> From: "Rafael Espíndola via llvm-dev" <llvm-dev at lists.llvm.org>
> |> To: "Steve King" <steve at metrokings.com>
> |> Cc: "llvm-dev" <llvm-dev at lists.llvm.org>
> |> Sent: Friday, December 11, 2015 4:28:33 PM
> |> Subject: Re: [llvm-dev] trouble hoisting GlobalValues
> |>
> |> On 11 December 2015 at 16:53, Steve King via llvm-dev
> |> <llvm-dev at lists.llvm.org> wrote:
> |> > Hello LLVM,
> |> > To reduce the code-size cost of relocations, I'm trying to hoist
> |> > GlobalValues that are used many times.  A new pass hides each
> |> > hoisted GV behind a BITCAST in the dominating BB.  The pass then
> |> > updates users with the output of the BITCAST.  This pass works
> |> > properly AFAICT.
> |> >
> |> > The problems come in instruction selection.
> |> > SelectionDAGBuilder::visitBitCast() treats the BITCAST as a no-op
> |> > and eventually users of the GV fold the relocation as if hoisting
> |> > never happened.  Experiments turning the BITCAST into a copytoreg
> |> > failed and anyway I feel like I'm just shooting in the dark.
> |> >
> |> > Can anyone can suggest a strategy for lowering a hoisted GV?  The
> |> > end result should be that the GV materializes as a simple
> |> > move-immediate to register.
> |
> |Also, something similar-sounding to this is done in
> |lib/Transforms/Scalar/ConstantHoisting.cpp
> |
> | -Hal
> |
> |>
> |> Nothing simple. One possibility would be to make constants explicitly
> |> materialized. Something like:
> |>
> |> https://github.com/apple/swift/blob/master/docs/SIL.rst#global-addr
> |>
> |> Which would also be useful for architectures where some constants are
> |> expensive to place in a register.
> |>
> |> Cheers,
> |> Rafael
> |> _______________________________________________
> |> LLVM Developers mailing list
> |> llvm-dev at lists.llvm.org
> |> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> |>
> |
> |--
> |Hal Finkel
> |Assistant Computational Scientist
> |Leadership Computing Facility
> |Argonne National Laboratory
> |_______________________________________________
> |LLVM Developers mailing list
> |llvm-dev at lists.llvm.org
> |http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev