[PATCH] D108360: [clang][NFC] Remove dead code

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 20 11:16:02 PDT 2021


rjmccall added a reviewer: Anastasia.
rjmccall added a comment.

Hi, Andy.  This patch is obviously okay, although it makes me wonder if it was introduced by a patch collision or something similar that needs closer attention.  CC'ing Anastasia.

I'm not sure that I agree with your overall plan, though:

- The WebAssembly operand stack is not a good match for an address space at the language level because it's not addressable at all.  If you can't meaningfully have a pointer into the address space, then you don't really need this in the type system; it's more like a declaration modifier at best.
- Allocating local variables on the operand stack ought to be a very straightforward analysis in the backend.  There's not much optimization value in trying to do it in the frontend, and it's going to be problematic for things like coroutine lowering.
- The security argument seems pretty weak, not because security isn't important but because this is not really an adequate basis for getting the tamper-proof guarantee you want.  For example, LLVM passes can and do introduce its own allocas and store scalars into them sometimes.  Really you need some sort of "tamper-proof" *type* which the compiler can make an end-to-end guarantee of non-tamper-ability for the values of, and while optimally this would be implemented by just keeping values on the operand stack, in the general case you will need to have some sort of strategy for keeping things in memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108360



More information about the cfe-commits mailing list