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

[Florian Hahn via Phabricator via llvm-commits]

fhahn requested changes to this revision.
fhahn added a comment.
This revision now requires changes to proceed.

Not sure if you are still interested in pushing this patch, but I think it would need to be re-based to work on the MemorySSA-backed DSE implementation, which is the default now. Also, as Eli mentioned there are missing correctness checks.



================
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
----------------
glider wrote:
> 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.
> Instead, we need to iterate over the constraints and pick all indirect outputs that don't have a matching input.

I am not sure I follow.Could you elaborate? As Eli mentioned, there probably are some additional legality checks missing. It would be good to start by adding some tests.  Also, we also need to account for the assembly using already assigned registers to read/write memory?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74853