@rjmccall @mstorsjo @aaron.ballman any advice on what to do next? Should I bring this discussion back to llvm-dev?

So, what could be the futur of that patch? Would that be okay to merge with a target_abi Clang attribute?

In D89490#2516255, @rjmccall wrote:

In D89490#2514695, @aguinet wrote:

I may be over-reacting to the way the patch seemed to be touching on the C++ ABI in multiple places. My understanding is that ms_abi is just a calling-convention attribute; it's basically "use the (default) calling convention that MSVC would use for this function". If that's all you want, then this is reasonable, although I am worried about creating a new attribute for every system that Wine chooses to target.

I literally based this patch on how ms_abi was implemented. It's unfortunately more than just teaching clang to change the calling convention on LLVM IR functions. The fact that ABI implementations are spread all over the place between various places in LLVM is, as far as I remember, a known problem discussed many times on llvm-dev, and looks like a hard one to fix.

Right, I understand that — I've even given an LLVM talk about it. I was just confused about your intent because you made some effort to match other parts of the ABI besides what we might traditionally consider the calling convention. I suppose some of those changes could be thought of as feature-specific calling conventions, but I don't usually think of them that way.

I may be over-reacting to the way the patch seemed to be touching on the C++ ABI in multiple places. My understanding is that ms_abi is just a calling-convention attribute; it's basically "use the (default) calling convention that MSVC would use for this function". If that's all you want, then this is reasonable, although I am worried about creating a new attribute for every system that Wine chooses to target.

In D89490#2482531, @rjmccall wrote:

I'm not calling Wine a niche use-case, I'm calling this feature a niche use-case. The lack of this feature has not blocked Wine from being a successful project. The feature has to stand on its own and be more broadly useful than the momentary convenience of a few developers.

In D89490#2481306, @rjmccall wrote:

I do feel like this is a terrible idea, sorry. It's a *very* niche use case to be dedicating a new compiler feature to, and it's a feature that demands a lot from the internal compiler architecture in ways that other features don't.

Rebase on current master, and take into account @aaron.ballman 's fixes (thanks!).

In D89490#2477937, @emaste wrote:

For now, only Linux/ARM64 is supported/tested.

Is there any reason this is Linux-specific (as far as support; I understand if it's not easy for you to test on non-Linux arm64).

Replace a just introduced const auto usage.

Clang format fixes

Rebased on current master branch, and added clang sema tests cc @aaron.ballman .

Thanks @aaron.ballman for the comments!

Multiple things:

Relax checks in CodeGenCXX/darwinabi-returnthis.cpp Clang test (to adapt to new attributes), and removes some useless brackets in if statements.

One missing formatting case...

Add some clang-format tags, and restore back the "original" formatting for these cases

Fix one clang-tidy warning

Update format + llvm tests

In D89490#2333073, @mstorsjo wrote:

I see that the llvm side of the patch lacks tests?

Thanks for the merge and reviews @alexshap (and @smeenai) !

This patch fixes one bug that was present: running llvm-lipo -archs on an LLVM IR object would crash, because we didn't check for that case in printBinaryArchs. This new patch fixes it, with an associated test.

@alexshap note that I don't have RW access to the llvm repository, so I can't merge this myself

Thanks for the new reviews! Fixed in the incoming diff

Fresh air!

Just a small variable naming cleanup

@alexshap there we go, it should fix all your remarks. Please let my know if there are still some issues.

Thanks @alexshap for your review and comments! Answers are inline.

Cleanup/refactoring

Clang-tidy + format remarks

Fix fatal error message of MachOUniversalBinary::ObjectForArch::getAsIRObject

Here is a new patch that includes:

• Update to monorepo and master
• Add forgotten RegisterMIRPasses.h
• Compile and test even in static mode

In D85740#2214477, @smeenai wrote:

About that part, like I said in the description, the way I see it is to enhance the ObjectYAML library so that we can generate LLVM bitcode file format (as described in https://llvm.org/docs/BitCodeFormat.html), so that yaml2obj can generate appropriate LLVM IR test objects. That would be used for instance to test the -create flag. Do you see another way to do this?

Can't you just use llvm-as to produce bitcode files and feed those to llvm-lipo?

Thanks for the comments. Answers below:

Use BUILDTREE_ONLY, as suggested by @serge-sans-paille .

Copyright added on new cpp files.

Cleaning up some commented CMake code...

Thanks Serge for the comments !

As a side remark, I have tested this with an out of tree MIR pass that dump MIR, and it works well. Should I add this test within the test suite? I didn't find such test for the current out-of-tree IR system (but chances are I missed it!).

BTW, I don't have any commit rights thought, so I guess someone needs to merge this at some point?

Thanks everyone for the review and comments!

Test case added!

I'll try to do the same with icc by tring the various ABI it supports (https://software.intel.com/en-us/node/522787), and getting the potential value with readelf (all with a script to be able to run this again!)

In D42350#983998, @aprantl wrote:

By the way, if you can contribute patches that add more 3rd party DWARF extensions to LLVM, that would be awesome to help avoiding clashes in the future.

In D42350#983933, @aprantl wrote:

I suppose the namespace for DW_CCs is large enough that doing this isn't very risky. Have you double-checked that this doesn't overlap with any other vendor extensions?

Hello Chill,