[PATCH] D91265: [PowerPC] Don't reuse the address of an illegal typed load for int_to_fp.

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.

The patch LGTM, but I really think the test case should change to more clearly convey the behaviour you desire to test.



================
Comment at: llvm/test/CodeGen/PowerPC/cvt_i64_to_fp.ll:15
+
+; AIX-LABEL: .postinctodbl:
+; AIX:  lwz [[MSW:[0-9]+]], 4(3)
----------------
Am I misreading this? It looks like the test is actually testing for the undesired behaviour. Namely, we load the value from the parameter pointer (presumably increment it there), store it to `stack - 8`, then load it from `stack - 8` into an FPR and convert.

Of course, this comment is tongue-in-cheek and I assume that the stores to the stack are **before** the increment and there are a pair of stores to the parameter pointer of the incremented value - but there is nothing in this test case to show that. Namely, this would equivalently match this wrong sequence:

```
lwz 5, 4(3)
addic 5, 5, 1
stw 5, -4(1)
stw 5, 4(3)
lwz 5, 0(3)
addze 5, 5
stw 5, -8(1)
stw 5, 0(3)
lfd 1, -8(1)
fcfid 1, 1
```

So my advice is to use the script to produce the checks as it is more maintainable and easy to prevent issues such as this. However, if you prefer not to use it, you should at least show the increment and the other stores to show that we are storing the incremented value to the parameter pointer and the non-incremented value to the stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91265