[polly] r287270 - [FIX] Do not try to hoist memory intrinsic

[Tobias Grosser via llvm-commits]

On Fri, Nov 18, 2016, at 01:47 PM, Johannes Doerfert via llvm-commits
wrote:
> On 11/18, Tobias Grosser wrote:
> > On Thu, Nov 17, 2016, at 11:11 PM, Johannes Doerfert via llvm-commits
> > wrote:
> > > Author: jdoerfert
> > > Date: Thu Nov 17 16:11:56 2016
> > > New Revision: 287270
> > > 
> > > URL: http://llvm.org/viewvc/llvm-project?rev=287270&view=rev
> > > Log:
> > > [FIX] Do not try to hoist memory intrinsic
> > > 
> > > Since we do not necessarily treat memory intrinsics as non-affine
> > > anymore, we have to check for them explicitly before we try to hoist an
> > > access.
> > 
> > Hi Johannes,
> > 
> > thanks for the fix! Would it be possible to add a test case or is this
> > hard for some reason?
> I did not have time to distill one from the SPEC benchmarks (and
> possibly others) that failed (with enabled hoisting) after the "memory
> intrinsics are affine patch". However, it should not be hard though. Any
> affine memory read that was caused by a memory intrinsic, e.g., memcpy,
> should do... 
> 
> I would guess that is sufficient to explicitly enabling hoisting for the
> memcpy testcase we already have to exposed the problem. Since initially
> there was no flag to disable hoisting, no test case explicitly enables
> it, thus the test coverage we lost (unit tests + lnt) by disabling
> hoisting is enormous...

No worries if you are busy. This can wait a couple of days.
 
> In my opinion the actual coverage problem is that hoisting is disabled
> globally on __all__ buildbots. Using __only__ profitable buildbots for a
> while caused us so much pain at some point. I would not have thought we
> would willingly go through the same trouble again but it seems history
> repeats itself.

Right. We really should not have had to turn this off, but unfortunately
this mode caused bugs in LNT, ffmpeg and -- as Zino told me on the LLVM
dev-mtg -- also on Android, which neither you nor me could commit to fix
at this time.

Now, I would be more than glad to turn this on again as fast as
possible. It seems we are already LNT clean. I am happy to check ffmpeg.

@Eli/Zino, do you at Qualcomm still see bugs/issues related to load
hoisting, which we should be aware of?

Best,
Tobias