The output from llvm-reduce still has significantly more attributes than
bugpoint does. Teach llvm-reduce to remove attributes.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Thanks for this! I'm always happy to see reduction tooling gain new powers :)
I've only touched these files in passing, so I'm probably not the best person to LGTM stamp. Just a few tiny nits.
llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp | ||
---|---|---|
2 | nit: ReduceAttributes.cpp | |
33 | nit: looks like const std::vector<T> & would match the std::function signature in runDeltaPass, and save us a copy? | |
35 | nit: indices are generally unsigned | |
49 | nit: int, since this function is returning an int? | |
llvm/tools/llvm-reduce/deltas/ReduceAttributes.h | ||
2 | nit: ReduceAttributes.h |
llvm/test/Reduce/Inputs/remove-attributes.py | ||
---|---|---|
6 | yikes! indentation |
llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp | ||
---|---|---|
35 | The comparison below against ChunksToKeep[ChunkIndex].end would be against an int. If I change AttributesIndex to unsigned, I get the following warning: ../tools/llvm-reduce/deltas/ReduceAttributes.cpp:41:26: warning: comparison of integers of different signs: 'unsigned int' and 'int' [-Wsign-compare] if (AttributeIndex == ChunksToKeep[ChunkIndex].end) ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ since Chunk::end is an int. (Though I don't think it makes sense for chunks to ever be negative. I could change the Chunk interface if it's not a yak shave, if you'd like?) |
Looks reasonable to me.
llvm/tools/llvm-reduce/deltas/ReduceAttributes.cpp | ||
---|---|---|
53 | Maybe include a matching TODO in extractAttributes? |
@nickdesaulniers are you planning on picking this up?
If not, just so we're done (or are we) with attributes, i can take over?
All yours, I didn't/don't have the time to sort out what the issue was on OSX, but if you find/fix/land it, I'll owe you a beer!
Clean-sheet implementation D83351 landed in rG03640ee0fa73c6eaf8cb12050203027239136789, and bots appear to be happy with it.
Abandon this?
yikes! indentation