Fix for PR14824: Optimization arm_ldst_opt inserts newly generated instruction vldmia at incorrect position

Stepan Dyatkovskiy stpworld at narod.ru
Mon Mar 25 09:20:03 PDT 2013


Hello Jiangning,
The main reason I did it, is that we need to analyze contents of MemOps 
*each time* we modify it: we need to prevent more than one "load" 
operations into the same place.
So, all "push-backs" and "inserts" should be preceded with analysis of 
what is inside MemOps already.
Did I miss something, may be? Currently, I concluded that Hao's patch 
should be inserted 3 times into the "LoadStoreMultipleOpti" code.
I've tried to make current patch simpler: I've removed MemOpsTracker 
class and reimplemented feature as private method. New version is 
attached to this post. In new patch each "push_back" preceded with just 
"CleanPrevLoadOpsInto(Reg, MemOps)" invocation.

Relative to "isIntersected". I looked for implementation of this in 
llvm-code and in STL, I've only found "set_intersection". But the last 
one is more complex than we need: it creates the new ordered set, that 
is result intersection. We don't need new set, we just need to check: 
are sets intersected or not. Though, may be.. do the  optimizers cut the 
part of "set_intersection" we don't need?

-Stepan.

Jiangning Liu wrote:
> Stepan,
>
> Looking at all codes in function ARMLoadStoreOpt::LoadStoreMultipleOpti,
> I think the newly added class MemOpsTracker seems to be splitting the
> change of MemOps for the 1st half and the 2nd half of this function,
> which would make this function hard to be understood.
>
> Why can't we simply add a new method/function to check the register
> overlap? For example, simply modify one line of Hao's code
>
> +            if (Reg == It->MBBI->getOperand(0).getReg()) {
>
> to be like,
>
> +            if (RegsAreOverlapped(Reg, It->MBBI->getOperand(0).getReg())) {
>
> Otherwise, I would feel your code change is a little bit over designed.
>
> Also, the function isIntersected looks strange, and why don't
> we naturally use a set to help this?
>
> Thanks,
> -Jiangning

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr14824-2013-03-21.patch
Type: text/x-diff
Size: 4488 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130325/641403d8/attachment.patch>


More information about the llvm-commits mailing list