[PATCH] D144454: Add builtin for llvm set rounding

This revision was automatically updated to reflect the committed changes. Closed by commit rG24b823554acd: Add __builtin_set_flt_rounds (authored by xiongji90). Changed prior to commit: https://reviews.llvm.org/D144454?vs=502507&id=503602#toc Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D144454: Add builtin for llvm set rounding

sepavloff accepted this revision. sepavloff added a comment. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454/new/ https://reviews.llvm.org/D144454 ___ cfe-commits mailing list cfe-com

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. Hi, @sepavloff Do you have any concern about this patch? Thanks very much. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454/new/ https://reviews.llvm.org/D144454 ___ cfe-c

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall accepted this revision. rjmccall added a comment. I have no objection to doing the documentation in a separate patch. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454/new/ https://reviews.llvm.org/D144454 __

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. Hi, @rjmccall and @sepavloff I updated built name to "__builtin_set_flt_rounds" as suggested and restrict the built to be used on x86, arm, aarch64 target. I did my local test on x86 and x86_64 and found arm and aarch64 code gen have already supported "llvm.set.round

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 updated this revision to Diff 502507. xiongji90 added a comment. Update the builtin name to __builtin_set_flt_rounds and restrict it to be used only on x86, x86_64, arm, aarch64 target. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall added a comment. Okay, so if we're adding this builtin, and it's not just imitating a stdlib function, we should definitely document it as a language extension. There's a section in the manual about controlling FP modes which is probably an appropriate place for this. I assume we nee

[PATCH] D144454: Add builtin for llvm set rounding

sepavloff added a comment. In D144454#4163688 , @rjmccall wrote: > I see. If we're going to take the target-independent values specified by > `FLT_ROUNDS`, then the original builtin name is more appropriate. Of course, > this has the disadvantage of n

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall added a comment. `__builtin_set_flt_rounds` seems better. We should add the portability checking, yeah. Look at the checking we do for certain builtins with `CheckBuiltinTargetInSupported` in `SemaChecking.cpp`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. In D144454#4163688 , @rjmccall wrote: > I see. If we're going to take the target-independent values specified by > `FLT_ROUNDS`, then the original builtin name is more appropriate. Of course, > this has the disadvantage of n

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall added a comment. I see. If we're going to take the target-independent values specified by `FLT_ROUNDS`, then the original builtin name is more appropriate. Of course, this has the disadvantage of not allowing target-specific values that might exist beyond those specified in the stand

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. In D144454#4163658 , @sepavloff wrote: > C standard function `fesetround` accepts rounding mode in the form of > target-specific values like `FE_TONEAREST`. They usually represent > corresponding bits in control register, so a

[PATCH] D144454: Add builtin for llvm set rounding

sepavloff added a comment. C standard function `fesetround` accepts rounding mode in the form of target-specific values like `FE_TONEAREST`. They usually represent corresponding bits in control register, so are different for different targets. `llvm.set_rounding` in contrast accepts rounding mo

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. In D144454#4162129 , @rjmccall wrote: > In D144454#4157717 , @xiongji90 > wrote: > >> In D144454#4142253 , @rjmccall >> wrote: >> >>> New buil

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 updated this revision to Diff 501732. xiongji90 added a comment. Change the builtin name to __builtin_fesetround Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454/new/ https://reviews.llvm.org/D144454 Files: clang/include/clang/Basi

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall added a comment. In D144454#4157717 , @xiongji90 wrote: > In D144454#4142253 , @rjmccall > wrote: > >> New builtins should be documented in the user manual. >> >> There are standard pragmas for changing

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. In D144454#4142253 , @rjmccall wrote: > New builtins should be documented in the user manual. > > There are standard pragmas for changing the rounding mode, right? What's the > interaction between the ability to set this dynam

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 updated this revision to Diff 501389. xiongji90 added a comment. Update test for llvm.set.rounding Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144454/new/ https://reviews.llvm.org/D144454 Files: clang/include/clang/Basic/Builtins.def

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 added a comment. In D144454#4142253 , @rjmccall wrote: > New builtins should be documented in the user manual. > > There are standard pragmas for changing the rounding mode, right? What's the > interaction between the ability to set this dynam

[PATCH] D144454: Add builtin for llvm set rounding

rjmccall added a comment. New builtins should be documented in the user manual. There are standard pragmas for changing the rounding mode, right? What's the interaction between the ability to set this dynamically with a builtin call and those pragmas? Comment at: clang/test

[PATCH] D144454: Add builtin for llvm set rounding

andrew.w.kaylor accepted this revision. andrew.w.kaylor added a comment. This revision is now accepted and ready to land. This looks good to me. I'm adding @rjmccall as a reviewer to make sure someone with front end expertise sees this. There seems to be a potential type mismatch between the int

[PATCH] D144454: Add builtin for llvm set rounding

xiongji90 created this revision. Herald added a project: All. xiongji90 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. llvm.get.rounding and llvm.set.rounding have been added to llvm and for llvm.get.rounding, a corresponding builtin "__b