[PATCH] D31164: [IR] Add AllowContract to FastMathFlags
Adam Nemet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 27 15:25:47 PDT 2017
anemet marked 2 inline comments as done.
anemet added inline comments.
================
Comment at: include/llvm/IR/Operator.h:177-178
public:
+ /// This is how the bits are used in Value::SubclassData so they should fit
+ /// there too.
enum {
----------------
spatel wrote:
> This comment isn't right - FMF is using "SubclassOptionalData" not "SubclassData".
>
> Not sure if it's worth mentioning as a code comment, but we only have room for one more FMF bit if we keep using SubclassOptionalData; it only has 7 bits total.
Yes, that is why I added the comment. You're right I mixed up the name of the field.
================
Comment at: include/llvm/IR/Operator.h:209-211
+ void setAllowContract(bool B) {
+ Flags = (Flags & ~AllowContract) | B * AllowContract;
+ }
----------------
spatel wrote:
> If we want to add a bool param, should we do that for all of the flags as a follow-up change? Seems odd to make setAllowContract() different than everything else if there's no change to use a 'false' setter in this patch.
Setting it with false is used in the clang patch in https://reviews.llvm.org/D31168.
But yes, I am planning to change all of these as a follow-on. I was going to put it in this patchset but the change is actually larger than I expected.
https://reviews.llvm.org/D31164
More information about the llvm-commits
mailing list