[PATCH] D115429: [Clang] Implement the rest of __builtin_elementwise_* functions.

Florian Hahn via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 21 12:11:31 PST 2021


fhahn added a comment.

In D115429#3201713 <https://reviews.llvm.org/D115429#3201713>, @junaire wrote:

> Hi, @aaron.ballman  I'm sorry for not updating the patch in time because I'm preparing for my school final exam :-(
> One thing I want to mention is that `__builtin_elementwise_roundeven` is actually been added in the RFC during the code review. You can find it in D111529 <https://reviews.llvm.org/D111529>.
>
>> "Prevailing rounding mode" is not super-useful, other than as a spelling for round-to-nearest-ties-to-even (IEEE 754 default rounding). Outside of a FENV_ACCESS ON context, there's not even really a notion of "prevailing rounding mode" to appeal to. I assume the intent is for this to lower to e.g. x86 ROUND* with the dynamic rounding-mode immediate.
>>
>> I would recommend adding __builtin_elementwise_roundeven(T x) instead, which would statically bind IEEE default rounding (following TS 18661-1 naming) without having to appeal to prevailing rounding mode, and can still lower to ROUND* on x86 outside of FENV_ACCESS ON contexts, which is the norm for vector code (and FRINTN unconditionally on armv8). I think we can punt on rint/nearbyint for now, and add them in the future if there's a need.

Yes, this was a change from the RFC after some feedback to the patch.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3137-3160
   case Builtin::BI__builtin_elementwise_ceil: {
     Value *Op0 = EmitScalarExpr(E->getArg(0));
     Value *Result = Builder.CreateUnaryIntrinsic(llvm::Intrinsic::ceil, Op0,
                                                  nullptr, "elt.ceil");
     return RValue::get(Result);
   }
+  case Builtin::BI__builtin_elementwise_floor: {
----------------
aaron.ballman wrote:
> It's starting to seem like we should have a helper function that takes the intrinsic to create and the name to give the operation; WDYT?
Sounds good! Might be good to split the `emitUnaryBuiltin` changes off into a separate change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115429



More information about the cfe-commits mailing list