[PATCH] D122918: [RISCV][CodeGen] Support Zfinx, Zdinx, Zhinx, Zhinxmin codegen

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:102
+        .add(MBBI->getOperand(1))
+        .addImm(MBBI->getOperand(2).getImm() + 4);
+  }
----------------
jrtc27 wrote:
> jrtc27 wrote:
> > Think about whether this is actually safe, please...
> I don't know why you've marked this as done; the line in question (which is now the line below) is not correct, the immediate could be 0x7f[cdef] at which point the arithmetic here will wrap to 0x80[0123] (which the machine verifier probably catches as out of range due to the immediate being signed?..). That is:
> 
> ```
> void
> foo(void *p, double d)
> {
>     *(double *)((char *)p + 0x7fc) = d;
> }
> ```
> will be miscompiled, surely.
Note this doesn't have to mean the store is to an unaligned location, either, p could be unaligned and then the addition of 0x7fc results in a well-aligned location. Though we want to support unaligned accesses anyway so that shouldn't be relevant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122918