llvm dev - Apr 2013 - [LLVMdev] Asking for a quick review

I've looked at the ExceptionDemo and the SysV AMD64 ABI and I'm unclear
on who, and when, the 'exception_cleanup' function is called. I have
some working exceptions but my cleanup routine is never called. In the
ExceptionDemo I turned on debug output and saw that the delete function
is also not called.


-- 
edA-qa mort-ora-y
-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
Sign: Please digitally sign your emails.
Encrypt: I'm also happy to receive encrypted mail.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130403/9edb102f/attachment.sig>

Hello all,

can someone do a quick review of the one-line proposal fix
attached to http://llvm.org/bugs/show_bug.cgi?id=15581?
I'm not sure that I can explain why it works, but it
(hopefully) is, so I really need a feedback with this.

Thanks,
Dmitry

Hi Dmitry,
> I'm not sure that I can explain why it works, but it
> (hopefully) is, so I really need a feedback with this.
I doubt anyone is going to approve the patch without being able to
explain why it works.

To me, it looks a little dodgy. Why should the sign of the offset
depend on its position in the instruction using it? I can vaguely see
there might be some argument that we have to be careful about the 4
variants "B+O", "B-O", "O+B", "O-B" but
the code a few lines later
seems to be trying to handle that ("if (SUB && OffsetIdx ==
1)").
That's where I'd be inclined to place my change if this bit of code
really is wrong.

At the very least the change is subtle enough that I'd want a clear
explanation in a nearby comment, I think. Hopefully that explanation
can make it clear the patch is correct too.

Also, there may be some misunderstanding about what's needed for a
small test-case. I think what Bill was after is a small .ll test-case
(as seen in test/CodeGen) which gets run whenever anyone runs "make
check" and tests this feature. It would be committed alongside the
change to the actual code.

Cheers.

Tim.