Skip to content

[WIP] PPC/PPC64: Fix ABI errors.#1680

Merged
redstar merged 1 commit intoldc-developers:ltsmasterfrom
redstar:ppcabi
Aug 14, 2016
Merged

[WIP] PPC/PPC64: Fix ABI errors.#1680
redstar merged 1 commit intoldc-developers:ltsmasterfrom
redstar:ppcabi

Conversation

@redstar
Copy link
Member

@redstar redstar commented Aug 13, 2016

Issue #1652 seems to be caused by ABI errors in the bootstrap compiler.
This PR updates the PPC/PPC64 ABI to conform to clang/gcc output.

@redstar redstar changed the title PPC/PPC64: Fix ABI errors. [WIP] PPC/PPC64: Fix ABI errors. Aug 13, 2016
@redstar
Copy link
Member Author

redstar commented Aug 13, 2016

This is not yet perfect. It fixes some errors but not all. Especially it is not possible to compile LDC 1.1.0 with this version.

@redstar redstar added the B-ppc label Aug 13, 2016
Issue ldc-developers#1652 seems to be caused by ABI errors in the bootstrap compiler.
This PR updates the PPC/PPC64 ABI to conform to clang/gcc output.
@redstar redstar merged commit f2430fd into ldc-developers:ltsmaster Aug 14, 2016
@redstar redstar deleted the ppcabi branch August 14, 2016 08:38
@kinke
Copy link
Member

kinke commented Aug 14, 2016

Is the omission of IntegerRewrite::isObsoleteFor() really necessary?

@redstar
Copy link
Member Author

redstar commented Aug 14, 2016

This is what clang seems to do. But I can check it.

@kinke
Copy link
Member

kinke commented Aug 14, 2016

IIRC, clang also does obsolete integer rewrites for System V. It's just that a LL struct with a single int/pointer is normally passed/returned identically to a rewritten integer. Omitting the rewrite in these cases just leads to a nicer and shorter IR and a shorter log file.

@redstar
Copy link
Member Author

redstar commented Aug 14, 2016

It seems to be different on PPC. From our test suite, using clang 3.7 on Linux/PPC64:

ldc_cabi2.cpp:

declare void @dretb1(%struct.B1* sret, i8) #0

With the obsolete integer rewrites in place I get for ldc_cabi1.d:

define void @dretb1(%ldc_cabi1.B1* noalias sret align 1 %.sret_arg, %ldc_cabi1.B1 %x_arg) comdat {

And without it:

define void @dretb1(%ldc_cabi1.B1* noalias sret align 1 %.sret_arg, i8 %x_arg) comdat {

I prefer to match clang as close as possible here.

@kinke
Copy link
Member

kinke commented Aug 14, 2016

As I said, as long as ldc_cabi1.B1 contains a single i8, there should be absolutely no difference (machine code wise) except for uglier IR.

I prefer to match clang as close as possible here.

Then we should do it consistently, either removing the obsoleteness check for all ABIs or doing it everywhere (as currently done for at least all 3 major ABIs I'm familiar with - x86 and x86_64). Needless to say, I prefer keeping it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants