[PATCH] D80683: [SelectionDAG] Fix up SimplifyDemandedBits for ppcf128

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai marked an inline comment as done.
nemanjai added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:855
+      !TLO.BitsRotatedForPPCF128) {
+    TLO.BitsRotatedForPPCF128 = true;
+    DemandedBits = DemandedBits.rotl(64);
----------------
RKSimon wrote:
> This is going to have the issue that it doesn't reset the flag to its old state when you leave the function, and TLO by design needs to be passed by reference.
> 
> Initially we might just have to bailout if we encounter MVT::ppcf128 until we can come up with a better solution?
I took a look at the calls to this function and was fairly certain that every time we do a non-recursive call to this, we will create a new `TLO` (which would set the flag to its initial state of `false`). However, maybe that is not the case (or not guaranteed to be the case in the future).

My initial thoughts on this were to reset the flag to `false` upon entering recursion (i.e. `Depth == 0`) but I ended up not adding that for the above reason.

I believe that if I add something like the following above this condition:
```
if (Depth == 0)
  TLO.BitsRotatedForPPCF128 = false;
```
we will achieve the desired result. Leaving the bit set after leaving the function isn't really an issue - as long as it is reset when we enter the recursive call chain.

Conversely, I can add another parameter to the function with a default value of `false` to signal that we have encountered a `ppcf128` and rotated the demanded bits. But I would favour the first approach I outlined above as it does not further complicate the API here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80683