Page MenuHomePhabricator

Remove Darwin support from POWER backend.
ClosedPublic

Authored by kbarton on Aug 20 2018, 12:20 PM.

Details

Summary

This is the first in a series of patches to remove darwin support from the PowerPC backend. This patch issues an error message if Darwin support is attempted. It also cleans up all existing test cases, either converting the test to use an alternative triple or removing the test if the coverage is no longer needed.

Once this patch lands, subsequent patches can be committed that actively remove the Darwin code paths from POWER backend. Since these paths will be dead code at that point, the cleanup can be done using post-commit reviews when applicable.

There is a companion patch for Clang, which fixes test cases in Clang. I will open another review for those changes and link to this revision.

Diff Detail

Repository
rL LLVM

Event Timeline

kbarton created this revision.Aug 20 2018, 12:20 PM

I personally can't think of a reason why not to do this. But I think sending an RFC to llvm-dev is in order here.

MatzeB accepted this revision.Aug 20 2018, 12:30 PM

I just realized that this is particular test is only about making the tests not use a darwin target anymore. That LGTM.

This revision is now accepted and ready to land.Aug 20 2018, 12:30 PM

Actually can you explain the removal of llvm/test/tools/dsymutil/PowerPC/sibling.test?

I personally can't think of a reason why not to do this. But I think sending an RFC to llvm-dev is in order here.

Thanks @MatzeB . I thought I had posted this some time ago, but I cannot find it now. I just sent an email.

Actually can you explain the removal of llvm/test/tools/dsymutil/PowerPC/sibling.test?

I had an email conversation with @JDevlieghere regarding this test. Someone suggested a test based on an old version of llvm-gcc from an old Powerbook G4, which is how this test was created. That was the only reason for this being a PowerPC test.

I tried generating an alternate test using a new GCC on a ppc64le machine, but the test caused an Assert failure in dsymutil. Jonas is looking at. In the meantime, he said this specific test should not block this patch.

If you have another idea for generating an equivalent test, I'm happy to do it.

Actually can you explain the removal of llvm/test/tools/dsymutil/PowerPC/sibling.test?

I had an email conversation with @JDevlieghere regarding this test. Someone suggested a test based on an old version of llvm-gcc from an old Powerbook G4, which is how this test was created. That was the only reason for this being a PowerPC test.

I tried generating an alternate test using a new GCC on a ppc64le machine, but the test caused an Assert failure in dsymutil. Jonas is looking at. In the meantime, he said this specific test should not block this patch.

If you have another idea for generating an equivalent test, I'm happy to do it.

I just think the commit message should mention why it was removed (no need for a replacement if you already discussed this with @JDevlieghere).

JDevlieghere accepted this revision.Aug 28 2018, 6:52 AM

Yup, The PowerPC aspect of that test is irrelevant. It was there because only GCC generates DW_AT_siblings and we resorted to using an old gcc-llvm to generate the test.

@kbarton Do you plan to rebase and commit this patch?

Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2019, 5:57 AM