On 03/18/2013 04:36 PM, Rafael Espíndola wrote:
Hi Rafael,

I don't think I should have to revert this patch.

I'm not even sure if there is another bug.

I don't know the code that is calling this. It's just my opinion that there
is some other issue.
Then you should investigate that. What we should never do is add

     if (!Fn) return; // should not happen

If it should not happen, this should be an assert (as it was before
your patch). It it is a logically valid condition, instead of the
wrong comment we should have a test where Fn is null.

Someone that knows clang can explain how FD is a function declaration but GV
is not a Function.
Why is that my responsibility to sort that out?
Because you changed the code.

This code is need for the mips test-suite to not regress and for my
attribute work to continue.
Sorry. You should not push problems to others. Looks like you actually
have a testcase and you just need to reduce it. Please revert this
patch and reduce the testcase. Depending on what you find you can put
this patch back, but without the comment and with a testcase where Fn
is null.

Reed


Cheers,
Rafael
THere are several patches that need to be reverted to set this back and we will just break other code and you will impede work that I'm doing that needs this.

I will look into what is causing this.

Give me some time to figure out what is causing this.

Please don't revert this. You will just make a lot of unnecessary work for me and the Mips team.

My original patch for attributes did not have this issue and I changed it to suite the reviewers. I did things exactly how the x86 port did them and they avoid this issue by luck or else maybe they
already found this problem and worked around it.

There is code already identical to this in this same .cpp for other targets.

We are talking about a two line patch.

You are punishing me for being honest about the situation.

I don't believe that I should be getting called in this situation with those parameter values. Maybe I'm wrong. There is no documentation on any of this so how would I know. Other code also assumes the same. I've filed a bug against clang for this and duly noted the issue both in the code and in the putback notes.

There are no mips target attributes other than mips16/nomips16 and those are for functions.

This test case has no Mips target specific attributes at all so why is this method even getting called?


Reed




_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to