[RFC] AEABI Divmod refactoring + 64-bit

[Renato Golin]

Hi Folks,

My patch to implement 32-bit divmod calls was basically a copy of the
LegalizeDAG DivRem lowering, with the case for return in registers. It
coped well with up to 32-bit values because there is no type legalization
to be done.


The Bad:

When I tried to add Custom for i64, I hit a wall. The problem seems to be
that LegalizeTypes can't cope with mixed lowering. (see divmod-custom.patch)

If I force Custom lowering for DIV/REM (by returning true
from CustomLowerNode(), it won't SetExpandedInteger() and the nodes that
depend on the result won't find it when they try GetExpandedInteger() from
it it.

If I return false, it will "expand" REM/DIV by calling ExpandIntRes_SREM()
which is basically lowering to a library call. Since my custom lowering is
in the ARM side, that lowering is actually wrong (and too early).

Example: i64 add ( srem ( %a, %b), %c )

If SREM is left as i64 for Custom lowering, ADD won't find Hi/Lo from its
results. If SREM is left to Expand, it'll turn into a library call and I
won't see it in Custom lowering later.

I believe the problem is that the LegalizeIntegerType is not expecting to
exit while there are still illegal types present in the DAG, which a 64-bit
REM would be, so there isn't much I could do, here. I also haven't found
any similar situation in other back-ends, maybe I didn't look in the right
place.

One (ugly) alternative would be to lower that to a pair of 32-bit REM/DIV,
which would be semantically wrong, but fixed later on by the custom
lowering. I fear that this could make the intermediate DAG unstable (for
things running between the type is "legalized" and the custom lowering
occurs).

Another ugly alternative is to recognize a library call to "divmod"
(via getLibcallName()) as a DIV/REM/DIVREM node. But I really don't want to
do that...


The Ugly:

Another approach that I was considering in the first place was to lower
everything on the generic LegalizeDAG. It does have the drawback that I'm
embedding ARM logic in there, but I tried to be as non-intrusive as
possible. (see divmod.patch).

To represent AEABI DivMod lowering, I created a TLI.usesRegistersForDivMod,
which is only true on ARM AEABI and false by default, and is *almost* only
used within the context of DIV/REM/DIVREM lowering. There's one ugly hack,
on ExpandLibCall(), accepting multiplicity of values + offset of return,
that I don't know how to work around, but that's just not right...

It's "generic" in the sense that other functions can use, but it's useless
for anything other than AEABI DivMod calls, and can lead to bad code later.
I really want to change that.


What Now?

So, the bad patch (divmod-custom) has a lot of copy, but so had the
original one. I'm not happy with that, but I'm ready to accept it as
collateral damage if I can legalize the type without automatically lowering
to a library call.

The ugly patch (divmod) is too ugly. Even though it works, it's more of a
proof-of-concept than a real proposal. I'm using it as a guide to the
custom lowering, to know where in the generic code I need changes. However,
the ugliest part is ExpandLibCall(), which if made right, it could make
this ugly patch "acceptable".

Is there an example of mixed custom+expand type legalization I can base my
implementation on?

Does anyone have other ideas on how to tackle this problem?

Thanks!
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130730/7393be49/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: divmod.patch
Type: application/octet-stream
Size: 18613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130730/7393be49/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: divmod-custom.patch
Type: application/octet-stream
Size: 8385 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130730/7393be49/attachment-0001.obj>