Details
- Reviewers
craig.topper pengfei lebedev.ri
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I guess i don't see why this is better than the current version of my patches?
Also, this has insufficient test coverage.
I don't consider this code to be better, and if it is considered to be,
i may need to reevaluate trying to contribute to this backend.
I'm sorry if I've caused you an issue here, but you did ask on D141877 that I show you my alternative approach, and I did my best to get something to you as fast as possible, despite me explaining that I'm stretched at the moment.
The level of refactoring that you employ in your version is more than is needed to add support for the SSE shifts to fix the regressions on D141778, and I don't agree that generalizing the code so much is necessary, especially as it affects understand-ability.
Maybe if you proposed refactoring patches that weren't part of a dependency chain for a simple improvement then it'd be easier to have a conversation.
I think the real problem is that i'm not doing these changes because i'm trying to fill a checkbox,
or generally improve things -- the yak will *never* be fully shaven, all of this code,
will be simply replaced with new-GISel + SMT long before that can fully happen.
I'm doing these because they are blockers to another change that is blocker to another change that is blocker to another change.
It's quite clear that very few people touch this code, and even fewer understand it,
so i was hoping that i could help unblock my own patches, instead of waiting for someone else to do that for me.
So yes, it's not really likely that such refactorings would appear without bigger purpose,
moreso because the review queue is already clobbered, and it's not good to further clobber it
with changes that don't help solve bigger problem.