[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:5114
+
+  ChangeStatus indicateOptimisticFixpoint() override {
+    return ChangeStatus::UNCHANGED;
----------------
Is it correct to say that an optimistic fix point is when all the "dynamic" components are set to default (`ieee`)? This attribute is reinventing what a fixed point means. Does this function need to set the optimistic fixed point? Or I would guess this function is actually never called, in which case it should be marked with `llvm_unreachable()`?


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:5118
+
+  ChangeStatus indicatePessimisticFixpoint() override {
+    return ChangeStatus::UNCHANGED;
----------------
Another theoretical issue with what a fixed point means for this attribute. Is there any sequence of actions where this function is called followed by something that depends on `isAtFixpoint()`?


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

https://reviews.llvm.org/D156129