[PATCH] Refactor sameNoopInput (determining tail-call legitimacy)

Evan Cheng evan.cheng at apple.com
Mon Aug 5 10:11:13 PDT 2013


On Aug 2, 2013, at 1:03 PM, Stephen Lin <swlin at post.harvard.edu> wrote:

> I suggested that because the semantics of the previous calls were
> probably not sufficient given how flexible calling conventions are.
> 
> We could also define that the target has to check this themselves.
> Right now, the targets do different things, but generally the pattern
> is that they only check if the arguments match if the calling
> conventions do not match.
> 
> If we codify that the target must check anytime that there is a
> possible issue with truncation, then that would be ok too, but I am
> just afraid that requirement would be lost to the ether without clear
> documentation of the assumption and cause strange bugs for someone
> down the line.

Fair point. I'm ok with the patch then.

Evan

> 
> Stephen
> 
> On Fri, Aug 2, 2013 at 11:58 AM, Evan Cheng <evan.cheng at apple.com> wrote:
>> I'm not crazy about allowTruncateForTailCalls(). IMHO, in general we should avoid adding *very specific* target hooks like this. If a target hook is absolutely required (looks like it is here), then we should at least try to make it more general so it may be useful for other optimizations. Do you think that's possible here?
>> 
>> Evan
>> 
>> On Jul 31, 2013, at 5:31 AM, Tim Northover <t.p.northover at gmail.com> wrote:
>> 
>>> Following Stephen's suggestion, I've added a TargetLowering function "allowTruncateForTailCalls" to replace the isTruncateFree call. Only three targets defined this before, so to be on the safe side I've only considered those three:
>>> 
>>> * X86 allows all truncates from 64-bit downwards (and relies on the previous code weeding out zeroext/signext attributes).
>>> * MSP430 didn't support tail calls anyway, so I left it at the default "nope".
>>> * Hexagon seems like it would allow truncates from i32 in general, and from i64 in its current guise. I've limited it to i32 due to fragility concerns.
>>> 
>>> This patch may have to change further if Nick's proposed weakening of the "no zeroext/signext" rule is accepted. I'll update it (with more tests) if that happens before this is committed.
>>> 
>>> http://llvm-reviews.chandlerc.com/D1216
>>> 
>>> CHANGE SINCE LAST DIFF
>>> http://llvm-reviews.chandlerc.com/D1216?vs=2999&id=3103#toc
>>> 
>>> Files:
>>> include/llvm/Target/TargetLowering.h
>>> lib/CodeGen/Analysis.cpp
>>> lib/Target/Hexagon/HexagonISelLowering.cpp
>>> lib/Target/Hexagon/HexagonISelLowering.h
>>> lib/Target/X86/X86ISelLowering.cpp
>>> lib/Target/X86/X86ISelLowering.h
>>> test/CodeGen/Hexagon/tail-call-trunc.ll
>>> test/CodeGen/X86/returned-trunc-tail-calls.ll
>>> test/CodeGen/X86/tail-call-legality.ll
>>> <D1216.2.patch>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list