[cfe-commits] r109960 - in /cfe/trunk: lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/exceptions.m test/CodeGenObjC/synchronized.m

[Chris Lattner]

Ok, that is certifiably awful.  If your approach works, then I remove my objections. :-P

-Chris

On Aug 3, 2010, at 4:54 PM, John McCall wrote:

> On Aug 3, 2010, at 4:16 PM, Chris Lattner wrote:
>> On Jul 31, 2010, at 5:47 PM, John McCall wrote:
>>> On Jul 31, 2010, at 5:38 PM, Chris Lattner wrote:
>>>> Inline assembly?  Yuck.  I thought that [llvm-]gcc just marked tons of locals as volatile?
>>> 
>>> It does, but then it ends up with all these horrible liveness problems it has to work around.
>>> It's also done in a really intrusive way in the type-checker.
>>> 
>>> It looked like my only bet for avoiding that without intrusive changes to IR gen — i.e.
>>> some way of saying "okay, these things are volatile even if the AST says they aren't" —
>>> was to go back over the function at the end and mark every single load and store
>>> as volatile.  I didn't really consider that acceptable.
>> 
>> I agree that the gcc implementation of this is terrible, but the end result (load/store operations being marked volatile) isn't so terrible.   What's wrong with a pass after IRGen that marks all accesses to allocas volatile?
> 
> 
> Because not all accesses to locals are direct.  For example, llvm-gcc gets this wrong because p's type after annotation is 'int * volatile', not 'int volatile * volatile':
> 
>  void opaque();
>  int main(int argc, char **argv) {
>    int x = 0, *p = &x;
> 
>    @try {
>      *p = 5;
>      opaque();
>    } @catch (id) {
>      return 1;
>    }
>  }
> 
> And of course you can do things like this:
> 
>  void add_two(int *x) { (*x) += 2; }
>  ...
>  int x = 0;
>  @ try {
>     add_two(&x);
>     opaque();
>  }
> 
> which then breaks under inlining, etc.  Even with inline assembly I'm not quite able to avoid all of this, even with a further fix I'm still testing, but I can get a lot closer than llvm-gcc can.
> 
> All the evil here springs from lowering to setjmp in the frontend.  setjmp was not designed to be injected into unsuspecting code like this;  doing so is orders of magnitude worse than my empty assembly constraints.  I am basically playing a game of chicken with the backend, particular since apparently nobody quite wants to admit that setjmp is a special case that they have to treat differently when, say, calculating liveness.
> 
> John.