[llvm-commits] PR6059 patch

[Rafael Espindola]

> There are several different issues going on here:
>
> * Simplifying llvm-gcc's target hooks for argument passing.
> * Correctness issues for ARM.
> * Code quality (i.e., the example above about simplifying code earlier)
>
> The changes you've been making so far to clean up argument passing are great.  It sounds like you've got some ideas for improving things further.  I've mostly been focused on correctness, and I think it is generally a good idea to separate out the issues involved in changes like this.  So, ignoring the other issues, here are a few comments on correctness:

They are not fully independent, as the proposed fix for correctness
simplifies the code. I am refactoring the ppc linux case to show that
it has a similar problem as the ARM one and can be fixed in the same
way.

If you are in a hurry, I can finish my ARM patch tonight. My plan was
to finishing refactoring the linux ppc case first.

> * Thinking about pr6059 made me realize that part of my change for pr5406 was completely unnecessary.  In one of your previously proposed patches, you suggested we revert that code, and I agree with you.  There is no need to check if i64 is a legal type when deciding whether to use it to pass aggregates.  The alignment is the only thing that matters.  I've gone ahead and reverted that.  It did not cause any regressions in the GCC compat tests for ARM APCS.

Have I? I noted that "So your patch was already a step in the right direction."

> * There is still a problem for pr6059, and it is more general than the original issue.  llvm-gcc now generates correct IR, but the backend is not putting the arguments in the correct registers.  I don't usually work with AAPCS so it's possible that I did something wrong when testing this.

The ABI is defined from C/C++ to assembly. The generated IL is an
implementation detail. If the FE uses i32, the BE can do anything with
i64. I am fine with adding padding, not adding padding, forcing it to
memory, asserting or making me coffee :-)


> * There is also an open issue for padding structures on big-endian systems.  For PPC, we use "byval" for arguments like this, even when those arguments are actually passed in registers, and the backend figures out where to put the padding.  Big-endian ARM APCS has the same issue, and it is just broken right now.  This really ought to be handled in the frontend, IMO.  While you're thinking about cleaning up the argument passing in llvm-gcc, you might consider something to address this.

I fully agree that this should be in the FE. I started refactoring the
linux ppc case but I am having a hard time getting qemu-ppc working
:-(

Do you know some other target I could test the big endian padding on?

Cheers,
-- 
Rafael Ávila de Espíndola