Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

setAlreadyVectorized does not delete obsolete metadata #21029

Closed
llvmbot opened this issue Aug 13, 2014 · 9 comments
Closed

setAlreadyVectorized does not delete obsolete metadata #21029

llvmbot opened this issue Aug 13, 2014 · 9 comments
Assignees
@rengolin
Labels
bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 13, 2014

Bugzilla Link 20655
Resolution FIXED
Resolved on Sep 01, 2014 05:35
Version trunk
OS Linux
Attachments Example input to opt that demonstrates bug
Reporter LLVM Bugzilla Contributor
CC @hfinkel,@rengolin

Extended Description

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.
@llvmbot
Copy link
Collaborator Author

llvmbot commented Aug 13, 2014

assigned to @rengolin

@rengolin
Copy link
Member

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.

@rengolin
Copy link
Member

Ended up refactoring the whole hint code... :)

http://reviews.llvm.org/D4913

@rengolin
Copy link
Member

Fixed in r215994

@rengolin
Copy link
Member

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.

@hfinkel
Copy link
Collaborator

hfinkel commented Aug 19, 2014

Is there a reason why a work-around would be difficult?

@rengolin
Copy link
Member

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.

@rengolin
Copy link
Member

rengolin commented Sep 1, 2014

Trying again on r216870 with MSVC2012-safe code.

@rengolin
Copy link
Member

rengolin commented Sep 1, 2014

It seems MSVC 2012 is happy with the code, thanks Seth Cantrell for the tip.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rengolin rengolin

Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@hfinkel @rengolin @llvmbot