[PATCH][ARM]Fix urem/srem of i64 for aeabi target (PR 17881)

Weiming Zhao weimingz at codeaurora.org
Mon Nov 11 17:44:48 PST 2013

PS: I grep 'RTLIB::SDIVREM_I64' in Target directory and only ARM defines

So the code shouldn't affect other existing targets.




Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


From: Weiming Zhao [mailto:weimingz at codeaurora.org] 
Sent: Monday, November 11, 2013 5:15 PM
To: 'Renato Golin'; 'Tim Northover'
Cc: 'llvm-commits'
Subject: RE: [PATCH][ARM]Fix urem/srem of i64 for aeabi target (PR 17881)


Hi Renato & Tim,

It's interesting to see there are already different approaches.
I actually did something similar with your patch #1 (custom lower DIVMOD64)
and I couldn't get it work either. 
The problem is legalize occurs earlier.
And another tricky thing is the second half of the return value
of__aeabi_divmod is needed. 
SDIV/UDIV is easier because no matter __adabi_uldivmod or  "__udivdi3" is
selected, the only difference is the function name. See
(LegalizeIntegerTypes.cpp::ExpandIntRes_SDIV(), where a i64 div is converted
to function call)
I read the other patch (devmod-legal) and seems OK. Why doesn't it work?
 You intercept it in ExpandIntRes_SRM too. 
You use getLibcallReturnSize/Value to check if a DivMod is called. 
My solution is check if SREMDIV is available, if it is, use it first. This
follows the same logic of how SREM_I32 works: if the target provides
RTLIB::SDIVREM_XX, then it will be used.
Although we changed LegalizeIntegerTypes.cpp, the logic actually follows the
way SDIV/UDIV works.  As long as a target doesn't fill , RTLIB::SDIVREM_I64
(default is 0 in TargetLoweringBase.cpp), they won't be affected.


Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


From: Renato Golin [mailto:renato.golin at linaro.org] 
Sent: Monday, November 11, 2013 3:38 PM
To: Tim Northover
Cc: Weiming Zhao; llvm-commits
Subject: Re: [PATCH][ARM]Fix urem/srem of i64 for aeabi target (PR 17881)


Hi Weiming,


I worked on this a bit in the past, and have attached both partial solutions
to your bug, as well as making my own bug a duplicate of yours, so at least
you can understand where we're coming from. Tim helped me review the patches
and understand the problem, so both of us will be reviewing this.





On 11 November 2013 22:03, Tim Northover <t.p.northover at gmail.com> wrote:

Hi Weiming,

> Please help to review the patch that addresses i64 mod operation for ARM
> aeabi.

Can this be done in ARM's ISelLowering? The problem is that ExpandRem
is *only* the correct implementation for the ARM-specific __aeabi_*
functions. The more generic GNU-style calls have a different

   int64_t __divmoddi4(int64_t l, int64_t r, int64_t *rem);

As it stands, any other target deciding to Expand divrem is likely to
get a nasty shock.


llvm-commits mailing list
llvm-commits at cs.uiuc.edu


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131111/8b7f91a2/attachment.html>

More information about the llvm-commits mailing list