[PATCH] D74853: [RFC WIP] Fix DSE for asm outputs (aka PR44913)

[Alexander Potapenko via Phabricator via llvm-commits]

glider marked 2 inline comments as done.
glider added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:291
+SmallVector <MemoryLocation, 4> getLocsForAsm(Instruction &I, const DataLayout &DL) {
+  // An inline asm() statement in C++ contains lists of input and output
+  // arguments used by the assembly code. These are mapped to operands of the
----------------
fhahn wrote:
> I didn't have time to take a close look yet, but I think the reasoning here should be based on semantics defined in the LangRef. It might also be necessary to improve the LangRef.
You are right, and my understanding that "=r" must always precede "=*m" didn't match LangRef.
For example, some inline assembly instructions in the Linux kernel, e.g.

```
{ i8, i32 } (i32*, i32, i32*, i32)* asm sideeffect ".pushsection .smp_locks,\22a\22\0A.balign 4\0A.long 671f - .\0A.popsection\0A671:\0A\09lock; cmpxchgl $3, $1\0A\09/* output condition code z*/\0A", "={@ccz},=*m,={ax},r,*m,2,~{memory},~{dirflag},~{fpsr},~{flags}"
```
 have the following constraints: `"={@ccz},=*m,={ax},r,*m,2,~{memory},~{dirflag},~{fpsr},~{flags}", where a memory output stands between two register outputs.

Instead, we need to iterate over the constraints and pick all indirect outputs that don't have a matching input.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:320
+  }
+  for (auto V : Outputs) {
+    int Size = DL.getTypeStoreSize(V->getType());
----------------
efriedma wrote:
> Iterating over a SmallSet of pointers is non-deterministic.
Correct, will fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74853