- User Since
- Feb 24 2019, 10:52 AM (239 w, 3 d)
Thu, Sep 14
No real comment on the issue itself but on the example as a former Eigen maintainer (sorry for the noise if that's all obvious for you):
Aug 23 2023
Aug 22 2023
Thanks, I've added a mir test that checks for fneg and gave it the name Matt suggested.
Aug 21 2023
Aug 14 2023
Jul 9 2023
Jul 8 2023
Aug 28 2020
Aug 21 2020
Does this conflict with D80791? Other than that it lgtm.
Aug 11 2020
Hi @chill, thanks for this patch, looks like the correct way to handle this.
Jul 3 2020
Apr 9 2020
Apr 7 2020
Apr 6 2020
Mar 12 2020
LGTM, but let's wait a bit for other comments.
Mar 11 2020
Feb 26 2020
Hi, thanks for this. The test case look very verbose and can probably be reduced. At least all unnecessary meta-information should be removed, e.g. does it really need all these function attributes?
Feb 10 2020
Jan 6 2020
Hi! Does this cover pre v8.3 pointer authenticating instructions (AUTIASP/AUTIBSP) too?
Dec 16 2019
Dec 11 2019
+1 to all comments above.
hasIllegalSPModification now only accepts matching sp modifying add and sub instructions of if they just increment/decrement sp.
Reopening this since it's still not correct.
Dec 9 2019
Reverse ping: Is this revision still active?
Okay, thanks for clarifying.
Dec 8 2019
Dec 5 2019
D69446 seems to introduce something similar. Are these patches related or is this just coincidence?
Thanks for correcting this comment. For possible future patches make sure to provide more context on your diff, i.e., run git diff -U99999. See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.
Dec 4 2019
@saugustine Thanks, I was just about to fix it with this patch but running the asan tests locally took longer than expected.
Thanks for the review!
Dec 2 2019
this looks good beside some minor inline comments.
Nov 23 2019
Add newline at eof
Nov 21 2019
Nov 20 2019
Address review comments.
Nov 12 2019
@ostannard The CFI instruction that can be omitted using this patch is the reason for D69097 (Return address signing for outlined functions) to be buggy, hence I've added you as a reviewer.
Nov 1 2019
@ostannard wow, good catch! Thanks for reverting this.
I'd actually prefer an option outliner-runs that is defaulted to 1 rather than a rerun option. From a user perspective I'd be unclear what outliner-reruns=0 exactly means? Does that mean one run or zero?
Oct 31 2019
Oct 30 2019
Thanks for the great review!
Oct 29 2019
- Disable outlining for functions that disagree on their v8.3A feature support
- Add a test for the above bullet point
- Improve support for v8.3a signing instructions
- Add tests
- Address review comments
- Moved signing logic into separate function
Oct 28 2019
Maybe I just don't see it but I don't get the reason for creating this new function doOutline. Doesn't it basically just call outline? Why can't you just add OutlinedFunctionNum to outline and then call outline in runOnModule?
Thanks for addressing the comments, LGTM!
Thanks for addressing the comments, LGTM.