How to suggest a very simple patch?

ParkHanbum · November 6, 2023, 4:11am

possibility of duplicate calls.

opened 05:34PM - 01 Mar 23 UTC

question

[`if (!L->isLoopSimplifyForm()) {`](https://github.com/llvm/llvm-project/blob/25…af6507e773f34a1b43f837eec918786619c545/llvm/lib/Transforms/Scalar/LoopFlatten.cpp#L384) in this line, it check this Loop whether simplify form or not. however, `isLoopSimplifyForm` doesn't seem to be needed because it's called inside the next condition check, `isCanonical`. see below call flow: ```cpp bool Loop::isCanonical(ScalarEvolution &SE) const { InductionDescriptor IndDesc; if (!getInductionDescriptor(SE, IndDesc)) return false; ... bool Loop::getInductionDescriptor(ScalarEvolution &SE, InductionDescriptor &IndDesc) const { if (PHINode *IndVar = getInductionVariable(SE)) return InductionDescriptor::isInductionPHI(IndVar, this, &SE, IndDesc); return false; } PHINode *Loop::getInductionVariable(ScalarEvolution &SE) const { if (!isLoopSimplifyForm()) return nullptr; ``` `isCanonical` -> `getInductionDescriptor` -> `getInductionVariable` -> `isLoopSimplifyForm` if `isLoopSimplifyForm` has returned false, `isCanonical` also returns false. so, I think it is not necessary first check whether this Loop `isLoopSimplifyForm` or not. how do you think?

I found two of duplicated call in code (need to review). but no one answer to my question. if they were correct that duplicated call, how to I suggest let this patch? thanks!

DavidSpickett · November 6, 2023, 9:10am

To submit your own patch:
https://llvm.org/docs/Contributing.html#how-to-submit-a-patch

DavidSpickett · November 6, 2023, 9:15am

And if the question is more, how can I suggest that someone else fix this, you could add a comment to ping the bug and see if that gets it some attention.

But really if you can, the best way is to submit a patch that removes these calls, and see if the reviewers disagree with doing that.

ParkHanbum · November 8, 2023, 6:43am

@DavidSpickett.
I wonder if LLVM has any limitations for short patches, like one-line patches. I have experience with gcc not accepting one-line patches.

Pierre-vh · November 8, 2023, 6:52am

As far as I know there is no limitation on how long/short a patch needs to be. I sometimes have to submit one-liners as well

ParkHanbum · November 9, 2023, 6:07am

@Pierre-vh thanks!!