[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