kpn added a comment.
I wanted to get the API straight before working on documentation.
================
Comment at: include/llvm/IR/IRBuilder.h:228
+ /// Enable/Disable use of constrained floating point math
+ void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+
----------------
erichkeane wrote:
> kpn wrote:
> > kbarton wrote:
> > > This is a minor quibble, but the method is setIsConstrainedFP, while the
> > > member is IsFPConstrained.
> > > I'm not sure if that is intentionally different, or an oversight.
> > Yeah, that's an oversight. Fixed.
> IS this fixed?
In my working copy, yes.
================
Comment at: include/llvm/IR/IRBuilder.h:234
+ /// Set the exception handling to be used with constrained floating point
+ void setDefaultConstrainedExcept(MDNode *NewExcept) {
+ DefaultConstrainedExcept = NewExcept;
----------------
mibintc wrote:
> andrew.w.kaylor wrote:
> > I think it would be better to add some enumerated type to describe the
> > exception semantic and rounding modes. The MDNode is an implementation
> > detail and an awkward one for the front end to have to deal with.
> I posted a patch showing the rounding and exception information being passed
> as enumeration not MDNode. I've uploaded the patch here,
> https://reviews.llvm.org/D62730
It's a good idea. D62730 beat me to the punch.
================
Comment at: include/llvm/IR/IRBuilder.h:1249
+ if (IsFPConstrained)
+ return CreateConstrainedFAdd(L, R, FMFSource, Name, nullptr, nullptr);
+
----------------
erichkeane wrote:
> Why set the last 2 to nullptr when you have defaults for these?
Eh, no really good reason.
================
Comment at: include/llvm/IR/IRBuilder.h:1259
+ Instruction *FMFSource = nullptr,
const Twine &Name = "",
+ MDNode *RoundingMD = nullptr,
+ MDNode *ExceptMD = nullptr) {
----------------
erichkeane wrote:
> The last 2 parameters are never actually used except in the test. Are these
> really important to have if they are never used in source?
They're part of the intrinsics. It seems like there should be a way to emit
them as part of the instruction when creating the instruction.
================
Comment at: include/llvm/IR/IRBuilder.h:1408
+ CallInst *CreateConstrainedFRem(Value *L, Value *R,
+ Instruction *FMFSource = nullptr,
----------------
erichkeane wrote:
> All these CreateConstrainedXXX should be distilled down to a single function
> that takes the intrinsic as a parameter.
That moves us further down the road towards having modes. I haven't seen a
front-end guy sign off on it yet, but it seems like a good idea.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53157/new/
https://reviews.llvm.org/D53157
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
