[llvm] r276942 - Don't invoke getName() from Function::isIntrinsic().

[Justin Lebar via llvm-commits]

Hi, Michel.

How do I run this test?

I suspect that the issue is @llvm.SI.load.const, because that's the
only builtin that's being touched here.  I'm not at my computer, but
is that actually a defined intrinsic?  If not, this patch would cause
isIntrinsic() to return false for that function, whereas before it
would return true (with intrinsic-id == not-an-intrinsic!).  If
something is checking isIntrinsic(), it could be changed to check
hasLLVMReservedName to get the same behavior we have today.

If this is causing problems for you, feel free to roll back.
Otherwise I'll sync up with either you or Matt in the morning
California time.

On Thu, Jul 28, 2016 at 12:25 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 28.07.2016 08:46, Justin Lebar via llvm-commits wrote:
>> Author: jlebar
>> Date: Wed Jul 27 18:46:57 2016
>> New Revision: 276942
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=276942&view=rev
>> Log:
>> Don't invoke getName() from Function::isIntrinsic().
>>
>> Summary:
>> getName() involves a hashtable lookup, so is expensive given how
>> frequently isIntrinsic() is called.  (In particular, many users cast to
>> IntrinsicInstr or one of its subclasses before calling
>> getIntrinsicID().)
>>
>> This has an incidental functional change: Before, isIntrinsic() would
>> return true for any function whose name started with "llvm.", even if it
>> wasn't properly an intrinsic.  The new behavior seems more correct to
>> me, because it's strange to say that isIntrinsic() is true, but
>> getIntrinsicId() returns "not an intrinsic".
>>
>> Some callers want the old behavior -- they want to know whether the
>> caller is a recognized intrinsic, or might be one in some other version
>> of LLVM.  For them, we added Function::hasLLVMReservedName(), which
>> checks whether the name starts with "llvm.".
>>
>> This change is good for a 1.5% e2e speedup compiling a large Eigen
>> benchmark.
>>
>> Reviewers: bogner
>
> This change broke the piglit test
> spec at arb_arrays_of_arrays@execution at ubo@fs-mixed-const-nonconst with the
> radeonsi driver, see the backtrace below and the attached LLVM IR.
>
>
> Thread 4 "si_shader:1" received signal SIGABRT, Aborted.
> [Switching to Thread 0x7fffebf67700 (LWP 30217)]
> 0x00007ffff56091c8 in __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
> 54      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
> (gdb) bt
> #0  0x00007ffff56091c8 in __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
> #1  0x00007ffff560a64a in __GI_abort () at abort.c:89
> #2  0x00007ffff5602107 in __assert_fail_base (fmt=<optimized out>, assertion=assertion at entry=0x7fffeee991e0 "PhysReg && \"Invalid SubReg for physical register\"", file=file at entry=0x7fffeee9908c "../lib/CodeGen/VirtRegMap.cpp", line=line at entry=442,
>     function=function at entry=0x7fffeee99b20 <(anonymous namespace)::VirtRegRewriter::rewrite()::__PRETTY_FUNCTION__> "void {anonymous}::VirtRegRewriter::rewrite()") at assert.c:92
> #3  0x00007ffff56021b2 in __GI___assert_fail (assertion=assertion at entry=0x7fffeee991e0 "PhysReg && \"Invalid SubReg for physical register\"", file=file at entry=0x7fffeee9908c "../lib/CodeGen/VirtRegMap.cpp", line=line at entry=442,
>     function=function at entry=0x7fffeee99b20 <(anonymous namespace)::VirtRegRewriter::rewrite()::__PRETTY_FUNCTION__> "void {anonymous}::VirtRegRewriter::rewrite()") at assert.c:101
> #4  0x00007fffedea977a in (anonymous namespace)::VirtRegRewriter::rewrite (this=this at entry=0x7fffdc020770) at ../lib/CodeGen/VirtRegMap.cpp:442
> #5  0x00007fffedeaaaab in (anonymous namespace)::VirtRegRewriter::runOnMachineFunction (this=0x7fffdc020770, fn=...) at ../lib/CodeGen/VirtRegMap.cpp:232
> #6  0x00007fffedd1c84c in llvm::MachineFunctionPass::runOnFunction (this=0x7fffdc020770, F=...) at ../lib/CodeGen/MachineFunctionPass.cpp:60
> #7  0x00007fffedb9bfc9 in llvm::FPPassManager::runOnFunction (this=0x7fffdc014620, F=...) at ../lib/IR/LegacyPassManager.cpp:1526
> #8  0x00007fffedb9c06b in llvm::FPPassManager::runOnModule (this=0x7fffdc014620, M=...) at ../lib/IR/LegacyPassManager.cpp:1547
> #9  0x00007fffedb9bc04 in (anonymous namespace)::MPPassManager::runOnModule (M=..., this=<optimized out>) at ../lib/IR/LegacyPassManager.cpp:1603
> #10 llvm::legacy::PassManagerImpl::run (this=0x7fffdc00abc0, M=...) at ../lib/IR/LegacyPassManager.cpp:1706
> #11 0x00007fffedb9bdde in llvm::legacy::PassManager::run (this=this at entry=0x7fffebf5ef60, M=...) at ../lib/IR/LegacyPassManager.cpp:1737
> #12 0x00007fffee90e691 in LLVMTargetMachineEmit (T=T at entry=0x69c5f0, M=M at entry=0x7fffdc002410, OS=..., codegen=codegen at entry=LLVMObjectFile, ErrorMessage=ErrorMessage at entry=0x7fffebf5f200) at ../lib/Target/TargetMachineC.cpp:205
> #13 0x00007fffee90ea1c in LLVMTargetMachineEmitToMemoryBuffer (T=T at entry=0x69c5f0, M=M at entry=0x7fffdc002410, codegen=codegen at entry=LLVMObjectFile, ErrorMessage=ErrorMessage at entry=0x7fffebf5f200, OutMemBuf=OutMemBuf at entry=0x7fffebf5f208)
>     at ../lib/Target/TargetMachineC.cpp:229
> #14 0x00007ffff0edf326 in radeon_llvm_compile (M=M at entry=0x7fffdc002410, binary=binary at entry=0x7fffdc000950, tm=tm at entry=0x69c5f0, debug=debug at entry=0x0) at ../../../../../src/gallium/drivers/radeon/radeon_llvm_emit.c:215
> #15 0x00007ffff0e13b6f in si_compile_llvm (sscreen=sscreen at entry=0x696850, binary=binary at entry=0x7fffdc000950, conf=conf at entry=0x7fffdc0009b0, tm=tm at entry=0x69c5f0, mod=mod at entry=0x7fffdc002410, debug=debug at entry=0x0, processor=1,
>     name=0x7ffff1001aaf "TGSI shader") at ../../../../../src/gallium/drivers/radeonsi/si_shader.c:6312
> #16 0x00007ffff0e17272 in si_compile_tgsi_shader (sscreen=sscreen at entry=0x696850, tm=tm at entry=0x69c5f0, shader=shader at entry=0x7fffdc0008c0, is_monolithic=is_monolithic at entry=false, debug=debug at entry=0x0)
>     at ../../../../../src/gallium/drivers/radeonsi/si_shader.c:6719
> #17 0x00007ffff0e27bed in si_init_shader_selector_async (job=job at entry=0x81d910, thread_index=thread_index at entry=1) at ../../../../../src/gallium/drivers/radeonsi/si_state_shaders.c:1160
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer