Add powerpc64 to compiler-rt build infrastructure.
Now that we have disabled the run-forever tests, and cleaned up the
intel 80-bit float based tests, we should be able to enable testing
compiler-rt for powerpc64.
Differential D36431
Add powerpc64 to compiler-rt build infrastructure. saugustine on Aug 7 2017, 2:48 PM. Authored by
Details
Diff Detail
Event TimelineComment Actions This patch appears to be perfectly fine. However, it triggers a large number of warnings. Namely, there's a large number of warning: ISO C forbids an empty translation unit [-Wpedantic] warnings produced. Also, I'm getting lots of warnings from compiler-rt/lib/builtins/atomic.c such as warning: implicit declaration of function '__c11_atomic_fetch_or' [-Wimplicit-function-declaration]. It would appear that the __c11_atomic_* family of builtins isn't exposed by the build compiler (and should presumably be declared in a header if file is being built with a compiler that doesn't expose those). Comment Actions What config are the buildbots using? I don't see these warnings in default or RELEASE style builds. I'm happy to fix them though.
I think it is better to mark plain powerpc (32) as unsupported in the testsuite, for similar reasons to marking powerpc64 unsupported. Whoever originally #ifdefed these did it in a way that is misleading. See the comments on D36249 for my reasoning.
What config do the buildbots use? I don't see these errors with DEBUG or RELEASE. I'm happy to fix them regardless, but I'd like to be able to anticipate these problems. Comment Actions Some buildbots use the distro gcc (looks like 5.4) and at least one uses gcc 7.1. I use gcc 6.2. Comment Actions I hope I haven't lost track of the patches that precluded this. If I remember correctly, all the X86 80-bit stuff was sorted out. We now know why those test cases were running forever (i.e. a vaarg function invoked as a non-vaarg function). So this just enables the truly generic builtins along with some PPC builtins. If my understanding is correct, I'd say this is ready to proceed (unless it was subsumed by one of the other patches). Long story short... LGTM. Comment Actions This has been sitting in approved state for more than 2 months. As far as I can tell, it wasn't committed. Do you plan to commit this soon or are you abandoning it for some reason? Comment Actions My apologies. I have gotten distracted by other things and will get this submitted shortly. |