User Details
- User Since
- Feb 5 2014, 1:36 AM (241 w, 2 d)
Yesterday
LGTM, thanks!
Wed, Sep 19
Thu, Sep 13
Before I review the code itself, have you agreed the .append_augmentation directive with the GCC/GAS developers to make sure we implement it the same way?
This looks like it has the same problem as D51418 (doesn't get applied to C++ static constructor functions).
Fri, Aug 31
Wed, Aug 29
Aug 17 2018
I think this needs additional tests to cover:
- The encoding of the CFI instruction in object files
- The assembly parser and printer changes, to make sure that IR->assembly->object will work the same as IR->object.
LGTM, thanks!
Aug 9 2018
LGTM, thanks!
Aug 8 2018
Aug 7 2018
Ont nit, other than that LGTM.
Aug 3 2018
Aug 1 2018
LGTM with one nit.
Jul 25 2018
Jul 18 2018
There are some variants of the PMULL instruction which depend on HasCrypto, which I think should be changed to HasAES.
Jul 13 2018
LGTM
Jul 12 2018
LGTM, thanks.
Jul 11 2018
Jul 4 2018
LGTM
Jul 3 2018
Jun 28 2018
It would be worth clarifying in the comments and commit message that this is the "Secure EL2" extension, to avoid confusion with the "Enhanced Support for Nested Virtualization" extension, which I assume will be in a later patch.
Jun 21 2018
This restriction is in the Armv6M architecture, so I think this should be done in ARMv6m, so it also applies to cortex-m1 and sc000, and I think this also applies to ARMv8mBaseline.
Jun 5 2018
It looks like we currently have two ways in which floating-point operands can get parsed, and this patch adds a third, which accepts values which overlap with both of the existing ones. Instead, this seems like a good time to tidy this up, always parsing to the same AArch64Operand, and using predicate functions to check if the value is valid for each operand type, as we do for integer operands with different ranges.
Apr 27 2018
Apr 26 2018
Apr 25 2018
LGTM
Apr 11 2018
Apr 5 2018
Apr 3 2018
This patch now changes 4 DAG nodes, but doesn't touch any tests for most of them. Are these changes NFC, and properly tested elsewhere? If not, this needs more tests.
Mar 6 2018
Mar 5 2018
Mar 1 2018
Feb 27 2018
LGTM, thanks.
Feb 21 2018
LGTM, but please wait a day to let other time-zones comment.
Feb 16 2018
Hi Simi,
Feb 14 2018
LGTM
The test case doesn't have to be exactly the -O0 output from clang, you should reduce it to clear IR which triggers the bug. To start with, are all of those allocas and memcpy calls required?
Feb 13 2018
LGTM, thanks
Feb 12 2018
To have a better handling than this, we'd have to error out earlier saying M-class doesn't have NEON, which probably needs more knowledge than that piece of code has...
Geoff Berry suggested to do this in DAG Combine
Ping (also for the related D42478).
Feb 9 2018
Add a triple to the test.