[PATCH] D55499: [PowerPC] intrinsic llvm.eh.sjlj.setjmp should not have flag isBarrier.

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a subscriber: craig.topper.
nemanjai added a comment.
This revision is now accepted and ready to land.

Since we have discovered this in the PPC back end but the issue also exists in ARM and X86, it'd be nice to notify @t.p.northover (already a reviewer) and @craig.topper about this potentially needing to change in their back ends as well.

In any case, other than the minor comments, the PPC portion LGTM.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstr64Bit.td:349
 } // hasSideEffects = 0
 
+let hasSideEffects = 1, usesCustomInserter = 1 in {
----------------
Please add a comment along the lines of:
`While longjmp is a control-flow barrier (fallthrough isn't allowed), setjmp is not.`

In both places where we define these (32 and 64 bit).


================
Comment at: llvm/test/CodeGen/PowerPC/sjlj.ll:163
+; CHECK-LABEL: test_sjlj_setjmp:
+; CHECK-NOT: bl _setjmp ; intrinsic llvm.eh.sjlj.setjmp does not call buildin function _setjmp.
+}
----------------
Put the comment on a separate line. I think this will make `FileCheck` actually look for all of that text which will clearly always make this test trivially succeed. Also, please correct the typo `buildin` -> `builtin`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55499/new/

https://reviews.llvm.org/D55499