[PATCH] D38088: Fix out-of-order stepping behavior in programs with hoisted constants.

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 24 09:00:56 PDT 2017



> On Oct 24, 2017, at 8:48 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Tue, Sep 26, 2017 at 2:05 PM Robinson, Paul <paul.robinson at sony.com> wrote:
> Doesn't need to be merged with the insertion point, I don't think? Does this merge across basic blocks? or just raise the uses of a constant to the beginning of a single basic block?
> 
>  
> 
> I read the pass as looking for a dominating insertion point across multiple basic blocks.  Doesn't mean the insertion point necessarily is in a different block, but I *think* it could be.
> 
> 
> Yeah, still seems like it might be nice to keep the location if it doesn't cross a basic block boundary.
> 
> Adrian - any ideas/thoughts on all this?

Looks like I missed the original review... Does Constant Hoisting just move a single constant to the top, or does it merge multiple identical constants (while hoisting them)? In the latter case getMergedLocation should definitely be used. In the former case I'm not so sure if erasing the location is the right thing to do in the first place (but it probably doesn't hurt much in practice). So I guess my question is: What are you suggesting should the location be merged with?

-- adrian


More information about the llvm-commits mailing list