[PATCH] [LoopVectorize]Teach Loop Vectorizer about interleaved memory access

Hao Liu Hao.Liu at arm.com
Tue May 19 03:45:41 PDT 2015


I've updated a new patch according to the comments from Renato and Adam. This patch adds a new class InterleavedAccessInfo to handle the analysis about interleaved accesses.

Review please.

Thanks,
-Hao


================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:401
@@ +400,3 @@
+
+    delete this;
+  }
----------------
rengolin wrote:
> Ouch, no!
> 
> If you do this, than a perfectly valid sequence like this:
> 
>     group.eraseFromMap(Map);
>     group.getDelta(); // there's no way to know that this won't work
> 
> will segfault.
> 
> A quick fix would be to leave the deletion to whomever is calling the function, so it's clear on the lifetime of the object:
> 
>     group.eraseFromMap(Map);
>     group.getDelta();
>     delete group;
>     group.getReverse(); // this is obviously wrong
> 
> But again, it seems to me that such logic is spread out too much. 
> 
> So a better fix would be to coalesce all of it into a class in itself. One that contains the maps, the groups, and the control over the lifetime of all its groups in a consistent way, without requiring the caller to know about it.
> 
> I'm ok with the temporary solution, as long as you add a FIXME to that effect.
That's reasonable.

I've added a new class InterleavedAccessInfo to manage this.

================
Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:670-671
@@ -488,1 +669,4 @@
+
+  /// Contains the relationships between the members and the interleaved access.
+  DenseMap<Instruction *, InterleaveGroup *> InterleaveGroupMap;
 };
----------------
anemet wrote:
> I've only started looking at this patch but I have a quick initial, more fundamental comment.
> 
> You are modifying an analysis pass (LoopAccessAnalysis).  Please add support for printing the new information gathered by the analysis.  Please also add tests under test/Analysis/LoopAccessAnalysis to verify the new information.
> 
> You may even want separate out the patch for the analysis part (LAA + tests) from the transformation parts (LV + tests + etc).
Hi Adam,

I've undated the patch to support printing the new information. I added more test cases as well.

I didn't separate patch as I think there are already a lot of history comments on both parts. Also I think the bound between analysis part and transformation part is clear.

Anyway, I can still separate this patch if you insist.

http://reviews.llvm.org/D9368

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list