LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 20655 - setAlreadyVectorized does not delete obsolete metadata
Summary: setAlreadyVectorized does not delete obsolete metadata
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC Linux
: P normal
Assignee: Renato Golin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-13 16:49 PDT by Arch D. Robison
Modified: 2014-09-01 05:35 PDT (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments
Example input to opt that demonstrates bug (858 bytes, application/octet-stream)
2014-08-13 16:49 PDT, Arch D. Robison
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arch D. Robison 2014-08-13 16:49:16 PDT
Created attachment 12893 [details]
Example input to opt that demonstrates bug

When pass loop-vectorize runs, it adds new loop metadata to indicate that the vectorizer ran, but fails to remove the old conflicting metadata.  For example, if you take the attached file for.ll and process it with:

    opt -S -loop-vectorize -debug-only=loop-vectorize for.ll

the output has:

    br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !3
    ...
    !1 = metadata !{metadata !"llvm.loop.vectorize.width", i32 1}
    !2 = metadata !{metadata !"llvm.loop.interleave.count", i32 1}
    !3 = metadata !{metadata !3, metadata !4, metadata !1, metadata !2}
    !4 = metadata !{metadata !"llvm.loop.vectorize.width", i32 4}

Note that !3 contains two conflicting widths: a width of 1 inside !2 and a width of 4 inside !4.  The pass should not have included !4 inside !3.

I'm using LLVM version 3.6.0svn.
Comment 1 Renato Golin 2014-08-13 17:07:55 PDT
Right, seems an vectorizer vs. O3+vectorizer issue, as with just -O3 the loop looks perfect.

Still, I think we could make setAlreadyVectorized() clean up after itself without needing additional cleanup passes. I'll have a look, should be simple.
Comment 2 Renato Golin 2014-08-14 15:42:24 PDT
Ended up refactoring the whole hint code... :)

http://reviews.llvm.org/D4913
Comment 3 Renato Golin 2014-08-19 12:34:02 PDT
Fixed in r215994
Comment 4 Renato Golin 2014-08-19 13:10:10 PDT
MSVC 2011 can't cope with my patch's C++11 goodness, so this will have to wait. Once MSVC 2013 becomes the new minimum requirement, I'll commit again.
Comment 5 Hal Finkel 2014-08-19 14:06:51 PDT
Is there a reason why a work-around would be difficult?
Comment 6 Renato Golin 2014-08-19 15:20:59 PDT
No reason, but the bug is very low priority, since it only happens in a particularly obscure combination of flags and even so, it's not really that critical.

I'd rather wait for a nice code than work around it now and forget later. :)

If this really affects people's work I can think of a work around, of course.
Comment 7 Renato Golin 2014-09-01 05:01:40 PDT
Trying again on r216870 with MSVC2012-safe code.
Comment 8 Renato Golin 2014-09-01 05:35:27 PDT
It seems MSVC 2012 is happy with the code, thanks Seth Cantrell for the tip.