This is an archive of the discontinued LLVM Phabricator instance.

Rename eh_sjlj_{setjmp|longjmp} intrinsic to lightweight_{setjmp|longjmp}
AbandonedPublic

Authored by MatzeB on May 7 2015, 5:01 PM.

Details

Summary

This should make it obvious that they are not used by llvms sjlj exception
handling code but to implement builtin_setjmp and builtin_longjmp.

This is a followup to http://reviews.llvm.org/D9313

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 25263.May 7 2015, 5:01 PM
MatzeB retitled this revision from to Rename eh_sjlj_{setjmp|longjmp} intrinsic to lightweight_{setjmp|longjmp}.
MatzeB updated this object.
MatzeB edited the test plan for this revision. (Show Details)
MatzeB added reviewers: john.brawn, rjmccall.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: Unknown Object (MLST).
rjmccall edited edge metadata.May 18 2015, 2:05 PM

I can't review changes to LLVM targets.

I agree with not naming the intrinsics after builtin_setjmp / builtin_longjmp, because using the word "builtin" in a list of intrinsics feels wrong, but please do mention in the documentation that these intrinsics implement builtin_setjmp / builtin_longjmp and are not their own new thing.

MatzeB updated this revision to Diff 26034.May 18 2015, 8:34 PM
MatzeB edited edge metadata.

New version which properly documents the intrinsics (also mentioning the intention to implement gcc builtin_setjmp/builtin_longjmp with them).

MatzeB updated this revision to Diff 26110.May 19 2015, 5:59 PM

Thanks for the review Justin, I update the documentation in the patch and the other comments mentioned in the review, backed out the setting of returns_twice for a different patch (which may be unnecessary), and fixed several places that had strange indentation after renaming.

ab added a subscriber: ab.May 19 2015, 6:43 PM

Shouldn't we AutoUpgrade the old intrinsics?

docs/LangRef.rst
8091

implement -> implemented

joerg added a subscriber: joerg.May 20 2015, 1:48 AM

I'd actually prefer if we didn't call them setjmp/longjmp at all. They have practically nothing to do with the libc functions and that's part of the confusion around them.

Putting this on hold as of Jörgs request, but that means http://reviews.llvm.org/D9313 should be accepted without renaming the intrinsic.

MatzeB abandoned this revision.Jul 10 2015, 11:46 AM