This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Factor out config prefix finding logic
ClosedPublic

Authored by Ericson2314 on Jan 2 2022, 11:32 PM.

Details

Summary

See the docs in the new function for details.

I think I found every instance of this copy pasted code. Polly could
also use it, but currently does something different, so I will save the
behavior change for a future revision.

We get the shared, non-installed CMake modules following the pattern
established in D116472.

It might be good to have LLD and Flang also use this, but that would be
a functional change and so I leave it as future work.

Diff Detail

Event Timeline

Ericson2314 created this revision.Jan 2 2022, 11:32 PM
Ericson2314 requested review of this revision.Jan 2 2022, 11:32 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 2 2022, 11:32 PM

Fix arg order error in Clang

Oh, Flang and LLD are the same. Just do them now then

Herald added a project: Restricted Project. · View Herald Transcript
Ericson2314 retitled this revision from [llvm][clang][cmake] Factor out config prefix finding logic to [llvm][clang][flang][lld][cmake] Factor out config prefix finding logic.Jan 3 2022, 12:20 AM

Get MLIR too

Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 11:18 AM
Ericson2314 retitled this revision from [llvm][clang][flang][lld][cmake] Factor out config prefix finding logic to [CMake] Factor out config prefix finding logic.Jan 3 2022, 11:18 AM
Ericson2314 edited the summary of this revision. (Show Details)

Oops, forgot some includes

lebedev.ri accepted this revision.Jan 7 2022, 11:46 AM
lebedev.ri added a subscriber: lebedev.ri.

Seems like a cleanup to me, and consistent with D116524.
Soft-accept, but only if D116524 is accepted first.

This revision is now accepted and ready to land.Jan 7 2022, 11:46 AM
beanz accepted this revision.Jan 7 2022, 11:56 AM

LGTM

This revision was landed with ongoing or failed builds.Jan 7 2022, 12:16 PM
This revision was automatically updated to reflect the committed changes.
sebastian-ne added inline comments.
llvm/CMakeLists.txt
205

Hi, adding this module path overwrites the llvm_check_linker_flag from llvm/cmake/modules/LLVMCheckLinkerFlag.cmake with the one defined in cmake/Modules/CheckLinkerFlag.cmake, which does not check the linker but the compiler.
This breaks our gcc+ld build because ld does not support --color-diagnostics.

Our downstream issue is here: https://github.com/GPUOpen-Drivers/llpc/issues/1645

Ericson2314 added inline comments.Jan 17 2022, 12:16 PM
llvm/CMakeLists.txt
205

Thanks for letting me know. Let's just rename the /cmake one then; nothing there is installed, on purpose, so we can change it freely without worrying breaking an interface used downstream.

I will make a patch for that in the next 2 days if no one beats me to it.

Ericson2314 marked an inline comment as done.Jan 17 2022, 10:38 PM
Ericson2314 added inline comments.
llvm/CMakeLists.txt
205

D117537 fixes this.