llvm dev - Aug 2012 - [LLVMdev] Illegal node introduced by DAGCombiner after legal phase

Hello,

I'm getting an instruction selection error because DAGCombiner is
introducing an illegal node after the legalizeDAG phase. Basically this is
what is going on:

1) During legalization, BR_JT gets expanded introducing a (mul x, 2).
2) After legalization  (AfterLegalizeDAG), that (mul x, 2) is converted to
an (shl x, 1).

However, that shl node introduced is illegal, and since my custom lowering
code won't run after this phase it gets into the instruction selector. I
would like to know if this is really a bug or it has to be handled by the
target code. I guess I could add a custom dagcombine hook for this node and
revert it in my target, but I want to hear what is the best thing to do.

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120826/92fcac4d/attachment.html>

Hi Borja,
> I'm getting an instruction selection error because DAGCombiner is
introducing an
> illegal node after the legalizeDAG phase. Basically this is what is going
on:
>
> 1) During legalization, BR_JT gets expanded introducing a (mul x, 2).
> 2) After legalization  (AfterLegalizeDAG), that (mul x, 2) is converted to
an
> (shl x, 1).
>
> However, that shl node introduced is illegal, and since my custom lowering
code
> won't run after this phase it gets into the instruction selector. I
would like
> to know if this is really a bug or it has to be handled by the target code.
I
> guess I could add a custom dagcombine hook for this node and revert it in
my
> target, but I want to hear what is the best thing to do.
it sounds like a bug in the DAG combiner.  I think checks for legal operations
tend to get added to the DAG combiner on an "as needed" basis, so
maybe no-one
had a platform for which this shift is not legal before.

Ciao, Duncan.

Borja,
In this situation, you need to find the place where shl is generated and add a
check to see if shl is legal before allowing it to do the transform.

Micah

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Borja Ferrer
Sent: Sunday, August 26, 2012 11:49 AM
To: llvmdev at cs.uiuc.edu
Subject: [LLVMdev] Illegal node introduced by DAGCombiner after legal phase

Hello,

I'm getting an instruction selection error because DAGCombiner is
introducing an illegal node after the legalizeDAG phase. Basically this is what
is going on:

1) During legalization, BR_JT gets expanded introducing a (mul x, 2).
2) After legalization  (AfterLegalizeDAG), that (mul x, 2) is converted to an
(shl x, 1).

However, that shl node introduced is illegal, and since my custom lowering code
won't run after this phase it gets into the instruction selector. I would
like to know if this is really a bug or it has to be handled by the target code.
I guess I could add a custom dagcombine hook for this node and revert it in my
target, but I want to hear what is the best thing to do.

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120827/b2fa446d/attachment.html>

Although I'm working on an out of tree target, this bug also happens for
the MSP430 target since we both share the limitation that shifts have to be
done in one bit at a time, that's why we need custom lowering.
Micah, I don't have commit access to fix it but the shl is getting
generated in line 1770 of DAGCombiner.cpp (inside the visitMUL function)
(fold (mul x, (1 << c)) -> x << c).

But there's something more about the design of how this works, if the
dagcombiner can potentially add illegal nodes after legalization, some of
these illegal nodes are going to be "cheaper" (shl vs mul) than not
performing the combine at all so maybe the custom lowering code could be
run again to catch these cases. I haven't thought too much about this, but
leaving a (mul x, 2) around doesn't sound too right.

Thanks for both replies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120827/8e2be56d/attachment.html>