This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Printf runtime binding pass
ClosedPublic

Authored by rampitec on Aug 30 2016, 9:13 AM.

Details

Summary

This pass is a port of the according pass from the HSAIL compiler.
It parses printf calls and setup runtime printf buffer.
After that it copies printf arguments to the buffer and fills in
module metadata for runtime.

Diff Detail

Event Timeline

alex-t updated this revision to Diff 69711.Aug 30 2016, 9:13 AM
alex-t retitled this revision from to Printf runtime binding pre-link pass.
alex-t updated this object.
alex-t added reviewers: b-sumner, tstellarAMD.
lib/Target/AMDGPU/AMDGPUPrinfRuntimeBinding.cpp
231 ↗(On Diff #69711)

Coding style, variables should begin with a capitcal letter. Here and several other places.

347 ↗(On Diff #69711)

Do we really need this function?

alex-t added inline comments.Aug 31 2016, 4:47 AM
lib/Target/AMDGPU/AMDGPUPrinfRuntimeBinding.cpp
347 ↗(On Diff #69711)

This one as well as "transPrintfVectorFormat" is only relevant to cpu device as a target.
It converts OpenCL printf format to that acceptable by libc printf.
I will remove both these functions if you confirm that LC is never expected to compile "fall through" path for CPU.

lib/Target/AMDGPU/AMDGPUPrinfRuntimeBinding.cpp
347 ↗(On Diff #69711)

We can't have X86 code in the AMDGPU backend, so these functions should be removed.

alex-t updated this revision to Diff 70154.Sep 2 2016, 6:46 AM

Changed according the reviewers comments

rampitec commandeered this revision.Aug 8 2019, 10:58 AM
rampitec added a reviewer: alex-t.
rampitec updated this revision to Diff 214193.Aug 8 2019, 11:01 AM
rampitec retitled this revision from Printf runtime binding pre-link pass to [AMDGPU] Printf runtime binding pass.
rampitec edited the summary of this revision. (Show Details)
rampitec added a reviewer: kzhuravl.

Cleaned, updated and rebased.
Pass does not require prelink infrastructure anymore.

This revision is now accepted and ready to land.Aug 10 2019, 8:49 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2019, 10:13 AM

Hi, MSan is complaining about this change:

28574==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x130075d in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::lowerPrintfForGpu(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:576:39
#1 0x12f8287 in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:613:10
#2 0x544a96d in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1750:27
#3 0x544a96d in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1863
#4 0x9e58b7 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:892:12
#5 0x7f84fa91f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#6 0x91ce99 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/opt+0x91ce99)

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34295/steps/check-llvm%20msan/logs/stdio

Hi, MSan is complaining about this change:

28574==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x130075d in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::lowerPrintfForGpu(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:576:39
#1 0x12f8287 in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:613:10
#2 0x544a96d in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1750:27
#3 0x544a96d in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1863
#4 0x9e58b7 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:892:12
#5 0x7f84fa91f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#6 0x91ce99 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/opt+0x91ce99)

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34295/steps/check-llvm%20msan/logs/stdio

Thanks! I will check it.

Hi, MSan is complaining about this change:

28574==WARNING: MemorySanitizer: use-of-uninitialized-value

#0 0x130075d in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::lowerPrintfForGpu(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:576:39
#1 0x12f8287 in (anonymous namespace)::AMDGPUPrintfRuntimeBinding::runOnModule(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:613:10
#2 0x544a96d in runOnModule /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1750:27
#3 0x544a96d in llvm::legacy::PassManagerImpl::run(llvm::Module&) /b/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1863
#4 0x9e58b7 in main /b/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:892:12
#5 0x7f84fa91f2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
#6 0x91ce99 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/bin/opt+0x91ce99)

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34295/steps/check-llvm%20msan/logs/stdio

Thanks! I will check it.

Hm... Is it only me, but msan has zillions of failures everywhere, starting with tablegen?

It's a bit hard to use, you need to build libcxx/libcxxabi with msan as well.
See here:
https://github.com/google/sanitizers/wiki/MemorySanitizerBootstrappingClang

I've started a build with track-origins here:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/14114
It could give more information.

It's a bit hard to use, you need to build libcxx/libcxxabi with msan as well.
See here:
https://github.com/google/sanitizers/wiki/MemorySanitizerBootstrappingClang

I've started a build with track-origins here:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/14114
It could give more information.

This is really complicated, I did not manage to do it in a short time. However, what was written on that line did not make any sense to me anyway.
Hence the change changing the loop, its iterator and the whole offended line: rL368645
I believe it should fix the issue.