Hi, I need fastmath flags support in LLVM dialect ops https://llvm.org/docs/LangRef.html#fast-math-flags
Here is my initial attempt https://reviews.llvm.org/D92485 where they are implemented just as array-of-strings attribute, but this approach has its issues.
Any ideas how they should like C++ api wise and in IR?
As I suggested on the review thread, independent UnitAttributes looks suitable. So does a custom attribute type backed by a bitfield. There are tradeoffs between these two.
C++ API should not be different from any other attribute.
An op interface would not be necessary unless one intends to process ops with FMF in an opaque way, e.g. without knowing what is the actual op.
It’s unclear to me how do you intend to target these ops in the LLVM dialect. If we need an equivalent in standard, this deserves a much more elaborate proposal.
It’s unclear to me how do you intend to target these ops in the LLVM dialect. If we need an equivalent in standard, this deserves a much more elaborate proposal.
For now I am going to add pass to our pipeline which runs after std->llvm conversion and sets these flags on all llvm ops based on some global option
As I suggested on the review thread, independent UnitAttributes looks suitable
To do this we need to add UnitAttr:$nnan, UnitAttr:$ninf, UnitAttr:$fast, ...etc to each op argument list (at least to LLVM_FCmpOp, LLVM_FloatArithmeticOp and LLVM_CallOp), is there any way to do this without copypaste? This will also generate build functions with 8 consecutive bool params.
Yes, one can always do something like
def FastMathFlags {
dag args = (ins UnitAttr:$nnan, UnitAttr:$ninf, ...);
}
def ...Op {
dag properArgs = (ins LLVM_Type:$lhs, LLVM_Type:$rhs, ...);
let arguments = !con(properArgs, args);
}
this can be further wrapper in classes to avoid concatenation in every op. OpBase.td and LLVM_OpBase.td are good source of inspiration of clever tablegen uses.
We can always improve ODS backend to properly handle trailing default values. This will, however, add a verifier that only the known attributes are used.
Also, how our build function for this should like? Function with 8 bool params is not very user friendly.
I am open to propositions 
We can have separate bitfield enum just to pass into build function, but it can be confusing to users to have separate enum just for this.
I’d recommend pulling (or reimplementing) swift::OptionSet up to LLVM’s ADT library, and using that. It is exactly what we’re looking for here, a typed/named collection of specific boolean options. We could then have ODS turn a “pile of unit attributes” into an OptionSet implicitly.
Just to make it clear, you use it in conjunction with an enum. There are a bunch of examples in this file, e.g.:
enum SemanticInfoFlags : uint8_t {
// Is the raw type valid?
HasComputedRawType = 1 << 0,
// Is the complete set of (auto-incremented) raw values available?
HasFixedRawValues = 1 << 1,
// Is the complete set of raw values type checked?
HasFixedRawValuesAndTypes = 1 << 2,
};
OptionSet<SemanticInfoFlags> SemanticFlags;
That would be a nice addition! “Bag of unit attributes” sounds like common pattern.
Is it similar to LLVM BitmaskEnum?
Looks similar, appears to lose the distinction between a set and a single enum, but it is hard to tell.
So, what’s the decision?
I’m fine either way: UnitAttr or BitmaskEnum. I suspect the latter will be more friendly for the build method and other APIs, but will require more effort on the printing/parsing helper needed (maybe a new TableGen type?).
Updated review. I went with custom attribute approach as there were no good decision how to handle build with bunch unit attrs.
Some more comments:
IntegerAttributeStorage for my bitfield attribute but had to do some ugly relative includes for AttributeDetail.h as it not part of public headers, any ideas how to do this properlyDefaultValuedAttr require constBuilderCall to be present but not enforces that and tblgen just silently generate invalid C++ code in that case