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

Renato Golin renato.golin at linaro.org
Thu Jun 4 13:25:56 PDT 2015


In http://reviews.llvm.org/D9368#183650, @anemet wrote:

> I am very surprised by this sentiment.  I would think that if reviews provide you with bugs, you'd be happy to fix them before the code going in.  (We effectively performed the bugfixing for you.)


I think you got me wrong. I *am* very happy that you are reviewing with such attention to detail...

> More importantly though, you're incorrect with your facts.  Existing paths in LAA were also modified by this patch which affect things like the LoopVectorizer which is on by default.


...but I was also (wrongly) assuming that the patch was still self contained. I stand corrected.

I agree with you that in the LAA case we need to make sure the behaviour outside of this patch should go unmodified.

> I agree that the review lasted longer than usual but I don't see the main reason being our "drastic code reviews".


I meant it in the good way.

> It would have been much easier if the patch was split up as I originally requested.  The existing modularity allowed for this but the patch didn't take advantage of it.  Also as we were finding correctness/design issues with the patch, Hao started improving the code (in actually very nice ways, thanks!!) which then needed new reviews again, etc. We're also in different time zones which didn't help either but I feel like the community should be able to put up with this much slack.


I'm fully aware of that and none of it was the reason why I asked for a breakpoint. As you said, Hao could have split the patch earlier, but once it was still small and self contained, the other bits would have been lost. This patch was re-written dozens of times and almost none of the original assumptions were kept in the final version. With such a game changer, it's hard to predict the future.

I just interpreted the last reviews as specific to the stride vectorizer, when (as you say), they're generic to LAA. My sincere apologies.

> Yeah, the LAA parts look good now.


Great! Round 2! :)

cheers,
--renato


http://reviews.llvm.org/D9368

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






More information about the llvm-commits mailing list