[PATCH] D25804: Fix 24560: assembler does not share constant pool for same constants

Zhao, Weiming via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 25 15:01:16 PDT 2016


Hi Renato,

Thanks for reviewing.

I didn't add new test case because existing test cases (modified in the 
patch) already exercise the logic. Is that OK?

Thanks,

Weiming


On 10/24/2016 3:14 AM, Renato Golin wrote:
> rengolin added a comment.
>
> In https://reviews.llvm.org/D25804#575584, @weimingz wrote:
>
>> Regarding the out-of-range test, give a test like:
>> foo1:
>>   PUSH {r4-r6,lr}
>>   LDR r7, =0x80808080
>>   .space 0x1000
>>   LDR r7, =0x80808080
>>
>> Existing llvm-mc gives "error: out of range pc-relative fixup value". Similarly, gnu-as gives "Error: invalid literal constant: pool needs to be closer"
>>   Is our case, if the first LDR is valid (within the range), then following LDR to the same constant should be valid as well since we always put CP at the end.
>
> I think this makes sense.
>
> The side effects of the caching are of *removing* constants, not adding them, so if anything, the range of the original load should be shorter, not longer. The number of instructions are also the same, so overall, this should never increase the code/data in any way.
>
> Also, whatever relocation didn't work before (negative, wrong range, wrong relocation), will keep not working, but that's nothing to do with the patch itself.
>
> cheers,
> --renato
>
>
> Repository:
>    rL LLVM
>
> https://reviews.llvm.org/D25804
>
>
>

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



More information about the llvm-commits mailing list