[PATCH] D156129: Attributor: Try to propagate concrete denormal-fp-math{-f32}

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:5120
+
+  bool isAtFixpoint() const override {
+    return Known.Mode.Input != DenormalMode::Dynamic &&
----------------
Some of my earlier comments are wrong, because I misunderstood the part about dynamic modes. But here's what I think needs to be consistent with the notion of a fixed point. The `update()` calls this function to check if it should attempt a refinement in yet another iteration. But the `updateImpl()` function tries to indicate when a fixed point is reached, which is when it is unable to follow through all the callsites anymore. There's a loss of information about whether future iterations should call `updateImpl()` again, because `indicatePessimisticFixpoint()` doesn't change any state.

In that case, maybe the state of this Attribute should really be a single bool called `AtFixpoint`? Then both optimistic and pessimistic calls set it to `true`, and return `UNCHANGED`, while `isAtFixpoint()` returns the current value of this bool.

Also, the introductory comments in Attributor.h should be updated to say that the Known/Assumed scheme is the most common, but not the only way to represent fixed points.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156129/new/

https://reviews.llvm.org/D156129