This is an archive of the discontinued LLVM Phabricator instance.

[LLD] Fix issue in HIP toolchain due to unspecified order of evaluation of the function object
ClosedPublic

Authored by aganea on Feb 8 2022, 12:46 PM.

Details

Summary

This fixes the issue raised in https://reviews.llvm.org/D108850#3303452

Before C++17, the function object is evaluated in a unspecified order. In the following example: https://godbolt.org/z/8ao4vdsr7 the function object is either evaluated before or after the arguments, depending on the compiler. With MSVC and /std:c++14 the function object is evaluated after the arguments; with clang and gcc, it is evaluated before. With C++17, the function object is guaranteed to be evaluated before the arguments, see: https://riptutorial.com/cplusplus/example/19369/evaluation-order-of-function-arguments

In our case, the issue was that the args conversion to ArrayRef as evaluated before the lambda call link, which internally was calling parseFlavor(), which in turned modified args. We ended with an ArrayRef argument that reflected the previous value of args.

Add coverage for -flavor which we didn't have before.

Diff Detail

Event Timeline

aganea created this revision.Feb 8 2022, 12:46 PM
aganea requested review of this revision.Feb 8 2022, 12:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 8 2022, 12:46 PM
yaxunl accepted this revision.Feb 8 2022, 2:00 PM

LGTM. Thanks.

This revision is now accepted and ready to land.Feb 8 2022, 2:00 PM
MaskRay added inline comments.Feb 8 2022, 3:38 PM
lld/test/ELF/amdgpu-duplicate-sym.s
10

Remove unneeded directives like .p2alignl, .fill, .size

21

remove

MaskRay accepted this revision.Feb 8 2022, 3:39 PM

Thanks!

aganea marked 2 inline comments as done.Feb 8 2022, 4:17 PM
aganea added inline comments.
lld/test/ELF/amdgpu-duplicate-sym.s
10

Both comments fixed in the commit, thanks for reviewing!