llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.69k stars 11.87k forks source link

CodeExtractor gives up on alloca used exclusively within extracted region #38789

Open vedantk opened 6 years ago

vedantk commented 6 years ago
Bugzilla Link 39441
Version trunk
OS All
CC @efriedma-quic,@hiraditya,@rnk,@sebpop

Extended Description

On Darwin, there is an extremely pervasive kind of cold code (error logging) which CodeExtractor has trouble with:

if.then3:                                         ; preds = %if.then
  %3 = tail call i8* @​llvm.stacksave(), !dbg !​5744
  %vla13 = alloca [2 x i8], align 16, !dbg !​5744
  %vla13.sub = getelementptr inbounds [2 x i8], [2 x i8]* %vla13, i64 0, i64 0
  store i8 0, i8* %vla13.sub, align 16, !dbg !​5750
  %numArgs.i = getelementptr inbounds [2 x i8], [2 x i8]* %vla13, i64 0, i64 1, !dbg !​5750
  store i8 0, i8* %numArgs.i, align 1, !dbg !​5750
  notail call void @​_os_log_debug_impl(... %numArgs.i ...)
  call void @​llvm.stackrestore(i8* %3), !dbg !​5752
  br label %if.end, !dbg !​5752

By default, extracting regions containing allocas is not enabled. In general this seems difficult to do. But here, all uses of the alloca are within the outlined region...

Can we do something simple to permit outlining in this scenario? Maybe check that the transitive closure of the alloca's uses all live within the alloca's block?

vedantk commented 6 years ago

Why is this alloca a VLA in the first place?

I'm not sure. I suspect it's because this logging routine was designed to work in environments where malloc isn't available.

LLVM doesn't really like dynamic allocas. If you can figure out how to have this not be a dynamic alloca, you might get better code all around.

If you do hoist the alloca, I'd recommend making sure you replace the stacksave and restore with lifetime markers to try to save stack space.

Thanks, that's a good idea.

rnk commented 6 years ago

Why is this alloca a VLA in the first place? LLVM doesn't really like dynamic allocas. If you can figure out how to have this not be a dynamic alloca, you might get better code all around.

If you do hoist the alloca, I'd recommend making sure you replace the stacksave and restore with lifetime markers to try to save stack space.

vedantk commented 6 years ago

I’ll send out a patch for this today. I tested it yesterday and found that it lets CodeExtractor successfully outline a lot of expensive logging code in some frameworks.

hiraditya commented 6 years ago

Seems very useful to implement. Let me know if you need any help. Thanks for bringing this example.

vedantk commented 6 years ago

The property you want to check is that the alloca is between a stacksave/stackrestore pair. That said, once you have code to detect that, you might as well just use it to hoist the alloca to the entry block instead.

Thanks Eli, that makes sense.

I suppose if we hoist the alloca to the entry block of the outlined function, we can eliminate the stack{save,restore} pair. I'll split that out into a different patch.

efriedma-quic commented 6 years ago

The property you want to check is that the alloca is between a stacksave/stackrestore pair. That said, once you have code to detect that, you might as well just use it to hoist the alloca to the entry block instead.

vedantk commented 6 years ago

assigned to @vedantk