Do you have more information about the change in Ubuntu that caused this to stop working?

Here is an AMDGPU patch that uses legalizeIntrinsic() https://reviews.llvm.org/D52923

I've implemented an alternative solution here that I think is a little more robust: https://reviews.llvm.org/D52861

In D51714#1248643, @smeenai wrote:

Is there anything holding this up?

I've created the 7.0.1 metabug, updated the merge-request.sh script. and created a merge request for this: https://llvm.org/PR39107

Use cmake's DEPRECATION message.

Is this more what you had in mind?

Updated. If you have any more feedback you can leave it on the review for the other patch.

Add custom install targets for modules.

I've just updated the patch with some fixes, can you try again?

Fix cmake failure with -DLLVM_LINK_LLVM_DYLIB=OFF

What CMake arguments were you using?

D51748 proposes refactoring this whole function and effectively makes the same change. Would you be able to test this to see if it helps your uses case?

Ok, my mistake.

There are a few other modules in the tree that are unnecessarily installed. So I made an attempt to fix this in a more generic way: D51748.

In D51714#1225545, @mgorny wrote:

Is LLVM_CONFIG dropped from cache here? I suspect the warning might fire for everyone who has LLVM configured.

Rebase on trunk.

In D51567#1222704, @chandlerc wrote:

I mean, sure.

I really don't know that supporting this ever expanding diversity of build strategies is worth its cost, but I don't see a specific reason to not take this patch....

My mistake. I had been working on the release_70 branch and did not realize the REQUIRES had been added to trunk when I rebased. Looks like this was fixed in r339253. I'll ask Hans to backport that to the stable branch.

@atrick does this look OK to you?

In D50668#1202755, @tstellar wrote:

I had to revert this, because it broke the Windows build. I am working on a new patch.

I had to revert this, because it broke the Windows build. I am working on a new patch.

In D50668#1198585, @philip.pfaffe wrote:

This makes complete sense to me, but I'm wondering whether add_llvm_loadable_module is the right macro to use here in the first place. All it does is call add_llvm_library with a MODULE argument, and set up the install.

So it seems to me that add_llvm_library(TestPlugin MODULE BUILDTREE_ONLY ...) should work just as well, without changing the setup. Does that make sense?

Use add_llvm_library() instead of add_llvm_loadable_module()

OK, that's good to know, I will drop this patch from Fedora.

Looks good to me.

One questions I had after looking into this bug was why does PPCTargetLowering::LowerFP_TO_INTForReuse() extend f32 values to f64 before creating the FCT* instructions?