Paths
| Differential D26321
[X86] Add LLVM version number for each intrinsic handled by auto upgrade for age tracking ClosedPublic Authored by craig.topper on Nov 4 2016, 10:53 PM.
Details Summary I've gone back and audited which release added which intrinsics to auto upgrade so we can track the age of these things better. I don't know if this this is exactly how we want it to be documented in the code, but at least its the information and a starting point.
Diff Detail
Event Timelinecraig.topper retitled this revision from to [X86] Add LLVM version number for each intrinsic handled by auto upgrade for age tracking. craig.topper updated this object. Comment Actions Thanks for the code archaeology, Craig! Comment Actions The structure looks OK. I don't think it matters if the upgrade code itself has changed after the commented version (e.g. older SSE upgrades were improved to support AVX+ intrinsics as well). I think we just need a comment at the top explaining our autoupgrade lifespan policy, but since the proposed changes to the version number increments I'm not sure what that still is.... Since we still have all that 3.X code there it obviously isn't at major increment. The only other thing that we may want to consider is if there is a way to add a -Wdeprecated style warning to soon to be deleted upgrade paths, but given that this is called in IR tools I don't know the best way to do that. Comment Actions So what is the autoupgrade lifespan policy? Was it ever discussed to remove an upgrade rule after it reaches some age? Comment Actions The original statement from Chris Lattner in the 3.1 timeframe was that we needed to keep them until we reached a merge version of 4.0 where we were allowed to break bitcode compatibility. With the more recent discussions on what major version 4.0 means, this original policy is no longer implementable since we are still required to maintain bitcode compatibility for a certain time period. I had a brief discussion with Chandler about this at the conference. He said we can try to propose a time based policy for this after 4.0 branches. He suggested we may either have a sliding window of X years back. Or alternatively some fixed point back greater than Y years and we would have to explicitly decide each time we wanted to move this point. zvi edited edge metadata. Comment ActionsThis change alone LGTM.
This revision is now accepted and ready to land.Nov 8 2016, 1:57 PM RKSimon edited edge metadata. Comment ActionsLGTM. I still think we need a comment of some kind to explain why each upgrade has a timestamp (even it has to say we don't have a policy atm), and I agree with Zvi that we need to raise this again on llvm-dev Closed by commit rL286933: [X86] Add LLVM version number for each intrinsic handled by auto upgrade for… (authored by ctopper). · Explain WhyNov 14 2016, 9:14 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 77942 llvm/trunk/lib/IR/AutoUpgrade.cpp
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||