In lots of places we were calling setDebugLocFromInst and passing
in the Builder member variable found in InnerLoopVectorizer. It
seemed neater to modify the existing function to take an Optional
IRBuilder argument that defaults to None, where None means use
the class Builder.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
- Removed the unnecessary extra interface and replaced the existing one to take an Optional instead.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
552 | If you make this the second operand, you can make it a default parameter, e.g. Optional<IRBuilder<> *> Ptr = None, so you don't need to pass None to each of the calls. |
Comment Actions
- Used @sdesmalen 's suggestion and swapped the operands around and made the IRBuilder<> option take a default of None.
Comment Actions
Thanks for the changes. Can you update the title and commit message (which are now out of sync with the updated patch)?
I also left a nit, but otherwise the patch looks fine to me.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp | ||
---|---|---|
552 | nit: Can you rename Ptr to CustomIRBuilder or something? |
Comment Actions
LGTM if you can address my nit about renaming the operand before committing. Thanks for the changes.
If you make this the second operand, you can make it a default parameter, e.g. Optional<IRBuilder<> *> Ptr = None, so you don't need to pass None to each of the calls.