User Details
- User Since
- Sep 23 2014, 3:21 AM (330 w, 1 d)
Mon, Jan 11
Sep 30 2020
Jan 28 2019
Jan 4 2019
@chandlerc I've reverted the patch r350402 sorry for causing the performance issue. I'll investigate the datalayout idea a bit more, @miyuki original solution to the function pointer aligment issue did something like that https://reviews.llvm.org/D44781
Dec 10 2018
Dec 6 2018
This was approved previously https://reviews.llvm.org/D55115 but when committed it caused failures on the sanitizer buildbots when building
llvm with clang (containing this patch). This is now fixed because I've added a check to see if getting the parent module returns null if it does then set the alignment to 0.
Dec 3 2018
Updated to use getPointerAlignment and removed test file that was reliant on compiler using assumptions about function alignment.
Nov 30 2018
Apr 26 2017
Looks fine, I think it's ready to land. Do you have commit access ? If not do you want me to commit on your behalf ?
Mar 31 2017
Mar 17 2017
Mar 6 2017
Mar 3 2017
Thanks for reviewing James.
Mar 2 2017
Mar 1 2017
Thanks for reviewing. Before I commit could you tell me if I need to update any build systems e.g. buildbots ?
Feb 28 2017
Feb 27 2017
This works for me, but the idiomatic macro usage in libc++, and (hopefully) libc++abi should always use !defined(FOO) as opposed to !FOO. I'll clean this up in the next week if nobody else wants to.
Feb 24 2017
Going for post-commit.
Ah sorry for the mistake. Fix is here https://reviews.llvm.org/D30343
Nov 17 2016
It's fine as a pass. I mean, it would be nice to include as part of some existing pass, but I'm not sure where it would fit in.
Nov 16 2016
Thanks for your review comments Eli.
Thanks for the review Tim, I really appreciate it.
Nov 15 2016
Nov 14 2016
Ping.
Nov 1 2016
Aug 18 2016
Jul 22 2016
Jun 16 2016
I'm surprised you didn't pick this problem before commit. Our buildbots seem happy, so this was definitely a downstream test.
Jun 15 2016
Jun 13 2016
Hi Renato,
Jun 10 2016
Thanks for reviewing Renato, will make your suggested changes before committing. Can you also review the clang part of the patch at some point http://reviews.llvm.org/D21179 ?
Jun 9 2016
Thanks for the review Renato. I replied back to your comments below.
LLVM part of the patch is here http://reviews.llvm.org/D21178
May 24 2016
Tim thanks for reviewing this patch. I've uploaded a new one with your suggested change. If I don't respond to any further comments after today it'll be because I'm on holiday and won't be back till next week.
LLVM part of the patch is here http://reviews.llvm.org/D20564
May 18 2016
http://reviews.llvm.org/D20394 which adds a test for the intrinsic in llvm
It's been our stance for a long time to require docs to approve changes, however small. I don't want to relax that which I think is a good constraint, not for such a seemly irrelevant issue.
I also doubt this will be the only addition in the new ACLE, so why not release the document, and then submit all changes then?
Or, maybe I am mistaken, and this is really that important... is it?
May 17 2016
Jun 29 2015
Hi Michael,
Jun 25 2015
Hi Michael,
Jun 24 2015
Thanks for the review.
Jun 23 2015
Hi Michael,
Hi Javed,
Jun 22 2015
Why are you testing for _ARM_FP if the core of what you're doing is adding +/- strings to -target-features? Why not test the target-features directly?
I don't think this will ensure the old code fails.
can someone have another look at this?
Thanks for the review Michael.
Jun 18 2015
Jun 17 2015
Thanks for the review.
Jun 15 2015
Can you explain in more detail how I can check for this internal bug using CHECK-DAG?
I'm assuming this is the target features, that normally come in the format:
+foo,-bar,+bazor
-target-feature +foo -target-feature -bar -target-feature +bazEither case, if you use:
CHECK-DAG: +foo CHECK-DAG: -bar CHECK-DAG: +bazThey could come in any order and it would still match.
The order that the features are specified on the cc1 command line won't mean that the features will be stored in the Features vector in the same order. This is because the features on the command line are parsed and stored into a StringMap then they're taken out again which can jumble up the order because StringMap's don't maintain order, below is the code which takes the features out of the StringMap in lib/Basic/Targets.cpp:
Changed 'HW_FP_remove =...' to 'HW_FP_remove |=...'
Jun 11 2015
TargetInfo::CreateTargetInfo takes the features from the command line and puts them into a StringMap and pulls them out again which can jumble the order of the features. So if I did write a test there's no guarantee that handleTargetFeatures will see them in the same order that I specified them on the command line. If you have any suggestions on how I can write a test for this then please let me know.
Jun 8 2015
Hi James,
Jun 3 2015
LGTM, but someone else should approve as well
LGTM, but someone else should approve as well