[flang-commits] [PATCH] D129316: Shift intrinsics

[Jean Perier via Phabricator via flang-commits]

jeanPerier accepted this revision.
jeanPerier added a comment.
This revision is now accepted and ready to land.

In D129316#3638582 <https://reviews.llvm.org/D129316#3638582>, @tarunprabhu wrote:

>> Is it necessary to return 0 for nonconformant calls, or would it be more efficient to omit the SelectOp's and just do a simple shift, where "whatever happens, happens" for nonconformant calls?
>
> I may have misunderstood @klausler  here (https://discourse.llvm.org/t/dealing-with-invalid-arguments-in-intrinsics/63630/4?u=tarunprabhu).
>
> I took his reply to mean that some degree of uniformity with other compilers is desirable although that might seem to incur a performance penalty in this case. I have limited experience with Fortran, and thus not best placed to comment on the preferred course of action. I am happy to modify this patch as others see fit.

I think your interpretation is correct in this case. If the behavior is unanimous on the SHIFT intrinsics, let's start by following it.
Please add a comment in the code explaining this choice here. Otherwise LGTM, thanks !



================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:3704
+  mlir::Value shift = builder.createConvert(loc, resultType, args[1]);
+
+  mlir::Value tooSmall = builder.create<mlir::arith::CmpIOp>(
----------------
Please add a small comment here that the "If SHIFT < 0 or SHIFT >= BIT_SIZE(I), the intrinsics will always return 0" behavior is not a Fortran requirement, but all other compilers do it.


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

https://reviews.llvm.org/D129316