- User Since
- Aug 22 2018, 6:51 AM (60 w, 1 d)
Respectfully, I've read all of that plus https://www.airs.com/blog/archives/189, and we've arrived at different conclusions. I'm fine with maintaining a local patch; I just wanted to point it out in case it was useful to others upstream.
I think the whole rounding step is questionable, not simply this change to it. As far as I can tell from researching this, the rounding down that occurs is for the starting address to place RELRO on a page boundary. The size of RELRO does not get rounded down, so rounding it up here by any amount risks making more data read-only than is necessary, which can lead to seg faults.
Craig, the pass still causes an 8% performance degradation for SPEC CPU 2017 imagick on skylake. Just FYI in case you want to investigate.
Aug 17 2019
Aug 16 2019
I've reverted temporarily. I see the error in the mlong-double-128.c file, just by looking at the diff above (the target checked in the ERR2 line is wrong after I changed the RUN line above), but I don't understand why the mlong-double-64.c test broke.
I believe this patch is causing 2 test failures on our x64 clang bots
@MaskRay - I have commit access now, so I'm just waiting on your approval that the patch is fine.
Aug 15 2019
I believe -mlong-double-80 is also rejected on other powerpc targets (fp80 is a x86 specific thing), so we can use a generic ELF platform.
Changed target for test.
Do you need me to change the test and commit for you? :)
Aug 12 2019
@MaskRay -- is this what you meant?
LGTM. As @MaskRay says, this shouldn't affect PowerPC, so you can remove the '[PowerPC]' from the title.
OK, I removed the other RUN line and the comment. I disagree with you about the comment, but I'm not going to hold up the patch for it -- I can keep it locally.
Aug 10 2019
OK, I moved it to a driver test. I agree that's more appropriate.
Aug 9 2019
Jul 18 2019
Hi, we just inherited this commit at Cray when we did our latest upstream merge and there are a few problems with it that I'd like to point out. Sorry that I was not part of the initial discussion here, but I didn't know that this work was being done and I had already done it for x86 in our downstream compiler a while ago.
Apr 1 2019
Could you simply rebase the patch, or acknowledge the new LLVM license, as your patch was written prior to the license change.
Mar 26 2019
I do not have commit access. I probably should request it at some point, but I'm fine with someone else merging this for me.
Mar 22 2019
This revision is now accepted and ready to land.Dec 3 2018, 12:55 PM
Nov 13 2018
I realize that you're probably striving for option compatibility with gcc, but continuing to name it -frecord-gcc-switches when it actually records Clang switches seems weird to me. It almost sounds like something that would dump gcc equivalents of all Clang options, or maybe let you know which Clang options you've used that match gcc options. Either way, by the name -- if you aren't familiar with the gcc option -- it doesn't read like it records Clang options.
Oct 29 2018
Aug 27 2018
Aug 22 2018
This is marked Accepted and is first in the sequence in D9375. Is there some reason this isn't merged yet? LGTM too.
Hi, I got here via llvm-dev => D9375 => D9403 (this) and have read through everything. I see that this patch has been stalled for about a year and I would like to know its status. Is it waiting on a resolution in LLVM for this problem that Jeroen mentioned on llvm-dev?