This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Do not internalize ASan device library functions.
ClosedPublic

Authored by hsmhsm on Sep 24 2021, 10:00 PM.

Details

Summary

ASan device library functions (those starts with the prefix __asan_)
are at the moment undergoing through undesired optimizations due to
internalization. Hence, in order to avoid such undesired optimizations
on ASan device library functions, do not internalize them in the first
place.

Diff Detail

Event Timeline

hsmhsm created this revision.Sep 24 2021, 10:00 PM
hsmhsm requested review of this revision.Sep 24 2021, 10:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 24 2021, 10:00 PM
hsmhsm retitled this revision from [AMDGPU] Do not internalize ASan functions. to [AMDGPU] Do not internalize ASan device library functions..Sep 24 2021, 10:02 PM
hsmhsm edited the summary of this revision. (Show Details)
pvellien added inline comments.Sep 27 2021, 1:00 AM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
520–522

there are two asan device functions that don't start with "__asan_*" prefix, they are the following:

  1. __sanitizer_ptr_cmp
  2. __sanitizer_ptr_sub

@b-sumner do we want to not internalize those functions?

llvm/test/CodeGen/AMDGPU/internalize.ll
3 ↗(On Diff #375013)

Since, this is a asan related change, I think it would be beneficial if we have a lit test with -fgpu-sanitize option.
Eg : https://reviews.llvm.org/D99071
please correct me if it is not needed here.

b-sumner added inline comments.Sep 27 2021, 10:12 AM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
520–522

Yes, I think sanitizer_* should be handled similarly to asan_*

b-sumner added inline comments.Sep 27 2021, 10:15 AM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
520–522

Sorry I forgot about the markup. I meant __sanitizer_* should be handled similarly to __asan_*

hsmhsm updated this revision to Diff 375469.Sep 27 2021, 9:50 PM

Fix review comments by Praveen and Brian, and rebase.

hsmhsm added a reviewer: tra.Sep 27 2021, 10:12 PM
hsmhsm removed a reviewer: tra.
yaxunl accepted this revision.Sep 28 2021, 12:20 PM

LGTM. Thanks.

This revision is now accepted and ready to land.Sep 28 2021, 12:20 PM
This revision was landed with ongoing or failed builds.Sep 28 2021, 6:49 PM
This revision was automatically updated to reflect the committed changes.

I've marked this test folder as needing AMDGPU in https://reviews.llvm.org/rG8f9f959b99e1.