[PATCH] D136939: [X86] Use default attributes for intrinsics

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D136939#3891815 <https://reviews.llvm.org/D136939#3891815>, @pengfei wrote:

> Third thought, we may need `noreturn` for those intrinsics that may raise exceptions under `maytrap` or `strict` cases. But I think we don't have a good way to switch the attributes with compiler options. Anyway, it is an irrelevant problem. And this patch LGTM.

Good point. The fact that the constrained FP intrinsics are currently marked as IntrWillReturn does seem incorrect to me. This can probably lead to miscompiles if a guaranteed-to-trap constrained FP intrinsic is followed by undefined behavior for example -- in this case, the undefined behavior will be propagated backwards past the trapping instruction. Unless I misunderstood something about the semantics of these intrinsics, we shouldn't be marking them as willreturn, and instead infer willreturn on call-sites for fpexcept.ignore (and maybe fpexcept.maytrap, I'm not clear on the precise constraints there.) Or as an alternative to inferring, handle them specially in `Instruction::willReturn()`.

In D136939#3891720 <https://reviews.llvm.org/D136939#3891720>, @pengfei wrote:

> @nikic IIUC, intrinsics without `IntrNoMem` don't need such change, right?

IntrReadOnly also needs to be adjusted to enable DCE. Other intrinsics can't be DCEd anyway, but can still benefit in other ways. E.g. lack of willreturn means that any "guaranteed-to-execute" optimizations will stop at the intrinsic, and lack of "nocallback" means that GlobalsAA will make pessimistic assumptions about access to internal globals. So it's best to use default attributes wherever possible, but IntrNoMem/IntrReadOnly are the most important ones.


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

https://reviews.llvm.org/D136939