llvm dev - Jul 2014 - [LLVMdev] LLVM commit 410f38e01597120b41e406ec1cea69127463f9e5

Hi,

I'm working on a target which have a variable size for CC (the same size as
the arguments). As a result getSetCCResultType, return a variable size.

In this commit, at the line DAG.getSExtOrTrunc(SetCC, DL, SelectVT), on my
target, you end up generating the Node you are replacing, and so creating a
loop in the DAG, which give a whole new meaning to the A in the acronym.
Subsequent code manipulating the DAG to not like it at all.

Can you explain me what you were trying to do in that commit ? I know it is
several month old, so the answer is likely not in cache, but that is
capital to me to understand what is the correct fix.

Thank,

Amaury SECHET
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140704/1b8fa2b3/attachment.html>

On Jul 4, 2014, at 8:18 PM, deadal nix <deadalnix at gmail.com> wrote:
> Hi,
> 
> I'm working on a target which have a variable size for CC (the same
size as the arguments). As a result getSetCCResultType, return a variable size.
> 
> In this commit, at the line DAG.getSExtOrTrunc(SetCC, DL, SelectVT), on my
target, you end up generating the Node you are replacing, and so creating a loop
in the DAG, which give a whole new meaning to the A in the acronym. Subsequent
code manipulating the DAG to not like it at all.
> 
> Can you explain me what you were trying to do in that commit ? I know it is
several month old, so the answer is likely not in cache, but that is capital to
me to understand what is the correct fix.
> 
> Thank,
> 
> Amaury SECHET
I was fixing creating a setcc with the wrong type for the operands for a target
with the same problem, which would then hit a selection failure later.

It was using getSetCCResultType on the result type of the SIGN_EXTEND node,
rather than the types being compared in the setcc. The new setcc needs to have
the right type, and then the result needs to be converted to the type of the
sign_extend.

I think in that case, it was something like (i64 sext (setcc (i32 x) (i32 y)).
getSetCCResultType is i64 for i64, and i32 for i32, so using the type of the
sext created i64 (setcc (i32 x) (i32 y)) which doesn’t work

-Matt

OK, so in you case, you want DAG.getSExtOrTrunc(SetCC, DL, SelectVT) to
tunc the result from i64 to i32 on 64 bits targets, if I understand
correctly.

2 questions:
 - Why not generating a selectcc node directly ? It avoid having to mess up
with intermediate values.
 - Why calling getSetCCResultType(VT) ? VT is not the type of a parameter
of setcc, and this looks incorrect to me.


2014-07-05 0:34 GMT-07:00 Matt Arsenault <arsenm2 at gmail.com>:
>
> On Jul 4, 2014, at 8:18 PM, deadal nix <deadalnix at gmail.com>
wrote:
>
> > Hi,
> >
> > I'm working on a target which have a variable size for CC (the
same size
> as the arguments). As a result getSetCCResultType, return a variable size.
> >
> > In this commit, at the line DAG.getSExtOrTrunc(SetCC, DL, SelectVT),
on
> my target, you end up generating the Node you are replacing, and so
> creating a loop in the DAG, which give a whole new meaning to the A in the
> acronym. Subsequent code manipulating the DAG to not like it at all.
> >
> > Can you explain me what you were trying to do in that commit ? I know
it
> is several month old, so the answer is likely not in cache, but that is
> capital to me to understand what is the correct fix.
> >
> > Thank,
> >
> > Amaury SECHET
>
> I was fixing creating a setcc with the wrong type for the operands for a
> target with the same problem, which would then hit a selection failure
> later.
>
> It was using getSetCCResultType on the result type of the SIGN_EXTEND
> node, rather than the types being compared in the setcc. The new setcc
> needs to have the right type, and then the result needs to be converted to
> the type of the sign_extend.
>
> I think in that case, it was something like (i64 sext (setcc (i32 x) (i32
> y)). getSetCCResultType is i64 for i64, and i32 for i32, so using the type
> of the sext created i64 (setcc (i32 x) (i32 y)) which doesn't work
>
> -Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140705/ea9c776e/attachment.html>