Hello Kai, all, I am not concerned solely about Darwin here, certainly we can make a patch to "fix" the problem there.
My concern is for the general state of this code: On 18 Sep 2014, at 22:26, Kai Tietz wrote: > it isn't true that I didn't replied to Iant. ( iains ;) ) > I did this on IRC. === a summary of the points I picked up from this irc conversation (and my interpretation). 1. There has been a change made "to make the upper path like the lower path" (as you said on IRC). - apparently (from our conversation) you don't expect this to be a general optimisation improvement. - but it doesn't seem to be either "obvious" or "a cleanup" to me - if you are asserting that it is a cleanup, then some explanation is in order. 2. Apparently you don't think it is necessary to have any testcase to demonstrate that the new code is working? - perhaps you are asserting that the code is "correct by inspection"? 3. You don't seem to think it necessary to amend the comments in the code to reflect the new functionality? 4. I find it difficult to accept that one can change a random part of the code, without any expectation of general improvement, that causes a breakage in some target - and then say "it's a target problem". ... I really think that these points need addressing - if you honestly believe that this code "only affects darwin" then surely you won't object to us changing it back to the previous working case. If you believe that the new version is benefiting any target, then please show me some (any) target that correctly exercises it. Iain. > As I > explained there already, this hunk about thunks is more consolidation > of code-paths in that function, and not really part of a feature. As > this code-path isn't prominent mark being Darwin-code - and please > don't take me wrong, but it seems to be until now the only target > reporting this issues - and therefore I strongly see the issue to be > solved for Darwin. I don't see that this changes needs an additional > testcase demonstration on a already regression-tested target that it > doesn't break ... This is somehow like asking for gcc-testcase > demostration that gcc's darwin target isn't responsible for earth's > warming ... > > Nevertheless I provided in the past already a patch which fixes the > issue well. As underlying issue seems to be that gotpcrel relocations > aren't suitable for call-instruction's address on Darwin's target. So > disallowing for this the use via the operand-check-predicate is the > right thing to do. As this prevents in all cases that for this target > such a construct might be generated even for none-thunk case, which > seems btw not being problematic at all. > > I don't agree to revert that patch. Please provide a testcase, why my > suggested fix isn't suitable.
