[PATCH] D40894: [XRay][compiler-rt+llvm] Update XRay trampoline CFI and register stashing semantics
Dean Michael Berris via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 29 10:21:05 PST 2018
dberris planned changes to this revision.
dberris added a comment.
In https://reviews.llvm.org/D40894#975323, @echristo wrote:
> A bit late, but I have some concerns here.
>
> I think the problem is that you were clobbering a bunch of registers that weren't "available" for you to clobber, but if you set this up as a normal calling convention they'll be saved - which I don't think you want to do. How about just saving all of the registers explicitly in your trampoline? If not, then you might just want to make it a normal call and save/restore over it? Given the goals of the work I'd probably lean toward the former. I think if you clobber all of the registers in call lowering you're just making the surrounding code worse?
>
> Thoughts?
I agree that we should try not to disturb the surrounding code when we lower the custom event instrumentation points. I'll update it to do all register stashing/restoration in the trampoline.
In https://reviews.llvm.org/D40894#975407, @echristo wrote:
> Also the register saves strike me as a somewhat odd optimization - you could also have just thrown this into SAVE_REGISTERS I think?
Good point. Yes, it's much cleaner that way, thanks!
https://reviews.llvm.org/D40894
More information about the llvm-commits
mailing list