With the support for changing the state of floating point optimizations
within a code block, we need to make sure that the function attributes
related to floating point math optimization are set in a conservative
way w.r.t. the body of the function to prevent illegal transformations
in the backends where such attributes are consumed.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
860 ms | libomp.lock::Unknown Unit Message ("") |
Event Timeline
Can we do this conservatively only if something is changed within the function definition, and otherwise respect the global settings?
I'm not sure I understand what you mean. The initial state is already the global settings coming from the language options (see constructor of CodeGenFunction).
Then whenever we process a unary/binary floating point operator we consider the local FPOptions and adjust the values of the function attributes accordingly.
Maybe instead of having an explicit state (the booleans) in CodeGenFunction tracking the values for these function attributes, I could simply update the function attributes directly. I guess the extra cost is negligible.
We have a very similar problem with the constrained FP intrinsics where the use of the intrinsics anywhere in a function requires (1) an attribute to be set on the function and (2) all the other code in the function to be emitted with intrinsics. What I've been treating as the plan of record is that we'll handle this by setting an attribute on the (AST) function definition whenever we see a relevant local pragma within it. And if we're doing that anyway, I think we can use it for this as well: set them in getDefaultFunctionAttributes and then clear them off if we see the attribute.
As for the RAII objects, my biggest concern here is the amount of extra work every time we emit an FP expression when we should have already set up the default configuration correctly. In the recent refactor, we started only storing FPOptions when there are non-default options for an expression. We should be able to use that to stop retranslating FPOptions to LLVM's options on every expression. (We can consider optimizing it for consecutive operations under the same non-default settings later if it proves important.)
Modify IR function attribute directly.
RAII object modifies IR builder state and function attributes only if needed.
Rebase.
As far as I can tell the failures reported shouldn't be caused by this change.
John, would you be able to land this on my behalf?