This is an archive of the discontinued LLVM Phabricator instance.

[LinkerWrapper] Emit assembly files from LTO with `save-temps`
ClosedPublic

Authored by jhuber6 on Jan 11 2023, 1:16 PM.

Details

Summary

Currently in LTO mode we don't emit any .s files for non-NVPTX targets
during LTO. This makes it diffcult to investigate any failures in the
assembly. This patch makes the save-temps mode output an assembly file
and then assembles it separately. I decided to simply invoke clang for
this as it would be a lot of effort to invoke the MCStramer interface
directly.

Diff Detail

Event Timeline

jhuber6 created this revision.Jan 11 2023, 1:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2023, 1:16 PM
Herald added a subscriber: inglorion. · View Herald Transcript
jhuber6 requested review of this revision.Jan 11 2023, 1:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2023, 1:16 PM
arsenm added inline comments.Jan 11 2023, 1:18 PM
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
531–537

can use initializer list?

Missing test?

jhuber6 updated this revision to Diff 488361.Jan 11 2023, 1:29 PM

Add test.

jhuber6 added inline comments.Jan 11 2023, 1:30 PM
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
531–537

Probably could, this is the same style we use in Clang.cpp so I usually end up using it.

jhuber6 updated this revision to Diff 488679.Jan 12 2023, 8:58 AM

Fix test, should be good to go now.

arsenm added inline comments.Jan 12 2023, 9:02 AM
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
531–537

well Clang.cpp is old, should move towards doing not old things

jhuber6 updated this revision to Diff 488690.Jan 12 2023, 9:21 AM

Changing to initializer list.

arsenm accepted this revision.Jan 12 2023, 9:26 AM

Thanks, maybe I'll actually be able to comprehend openmp now

This revision is now accepted and ready to land.Jan 12 2023, 9:26 AM
This revision was landed with ongoing or failed builds.Jan 12 2023, 10:55 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Jan 12 2023, 5:53 PM

Looks like this breaks tests on Windows: http://45.33.8.238/win/73276/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Looks like this breaks tests on Windows: http://45.33.8.238/win/73276/step_7.txt

Please take a look and revert for now if it takes a while to fix.

Yes, seems I forgot that clang is clang.exe on Windows. I'll fix it real quick.

Thanks! Still failing with that though: http://45.33.8.238/win/73278/step_7.txt

Thanks! Still failing with that though: http://45.33.8.238/win/73278/step_7.txt

Hm, I'm unsure what would be causing that. I could always just disable the test on Windows. This program isn't really used on Windows anyway.