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
Diff Detail
Event Timeline
- 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. |
- Used @sdesmalen 's suggestion and swapped the operands around and made the IRBuilder<> option take a default of None.
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? |
LGTM if you can address my nit about renaming the operand before committing. Thanks for the changes.
Is this interface still needed? I expected that the implementation could now use its Builder member variable.