[PATCH] Constant Hoisting Pass

[Eric Christopher]

On Thu, Jan 23, 2014 at 3:22 PM, Juergen Ributzka <juergen at apple.com> wrote:
> Hi Paul, Eric, and Adrian,
>
> the constant hoisting pass is only enabled starting with optimization level
> -O1 - the same as for CodeGenPrepare. It won’t interfere with -O0 debug
> builds.

I wasn't too worried about that, however, keep in mind that debug
info, in particular location information is actually used extensively
at optimization levels as well. I.e. look at something like AutoFDO
which is using debug information plus sampling to get hot spots in the
program so it can be optimized.

>
> I also don’t want to make the debugger life more difficult and after
> checking with Adrian I updated the patch to also add debug locations (I just
> simply forgot them).
> I just copy the debug location from the instruction I generate the
> rematerialization code for, because they belong together. For the hoisted
> constants I don’t have a single debug location anymore to pin them to, so I
> just use the debug location from the following instruction.
>
> What you guy think?
>

For the hoisted constants I'd prefer you pick the instruction of the
earliest such use of the constant in the function if possible. That
will a) keep it closer, and b) hopefully match up with some of the
uses.

Thoughts?

-eric

> Cheers,
> Juergen
>
>
>
>
> On Jan 23, 2014, at 11:26 AM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
>
> -----Original Message-----
> From: Adrian Prantl [mailto:aprantl at apple.com]
> Sent: Thursday, January 23, 2014 11:13 AM
> To: Robinson, Paul
> Cc: Juergen Ributzka; LLVM Commits; Owen Anderson; Eric Christopher
> Subject: Re: [PATCH] Constant Hoisting Pass
>
>
> On Jan 22, 2014, at 18:05, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
>
> With this kind of thing, I get concerned that moving stuff around will
>
> cause unnecessary chaos with debug info.  But it looks like the new
> instructions you're creating don't have a source location attached?  In
> this case that's probably the right thing to do.  Let me know if I'm
> misunderstanding what's going on here.
>
> Not attaching any location will create a hole in the function, which is
> also not ideal. I've seen this cause interesting problems with inlining
> where the debugger then has no chance to know which of the abstract
> subroutines an instruction belongs to and stepping totally breaks.
>
>
> Hm.  Machine instructions with no explicit location will implicitly
> inherit the location of the preceding instruction.[*]  Is that not what
> happens with IR instructions?
> --paulr
>
> [*] This is not great when the preceding instruction is in a different
> block, and I've poked at that from time to time. But is a digression from
> this discussion.
>
>
> If there really is no useful location for an instruction, IMHO it should
> get a location pointing to line 0 + a valid scope that belongs to the
> function. In Clang we have ArtificialLocation for this which is used for
> the helper functions emitted for Blocks in Objective C.
>
> -- adrian
>
>
>