-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
i8 bool accumulator not converted to i1 #57448
Comments
We'd want to push the Not sure what an elegant way to fix this would be cc @rotateright |
Candidate patch: https://reviews.llvm.org/D134954 |
nikic
added a commit
that referenced
this issue
Oct 4, 2022
Reapply with a fix for the case where an operand simplified back to the original phi: We need to map this case to the new phi node. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes #57448, which was my original motivation for the change.
nikic
added a commit
that referenced
this issue
Oct 5, 2022
The infinite loop seen on buildbots should be fixed by 1189770 (assuming there are not multiple infinite combine loops...) ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes #57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
nikic
added a commit
that referenced
this issue
Oct 7, 2022
Relative to the previous attempt, this adjusts simplification to use the correct context instruction: We need to use the terminator of the incoming block, not the original instruction. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes #57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
nikic
added a commit
that referenced
this issue
Oct 17, 2022
Relative to the previous attempt, this is rebased over the InstSimplify fix in ac74e7a, which addresses the miscompile reported in PR58401. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes #57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
sid8123
pushed a commit
to sid8123/llvm-project
that referenced
this issue
Oct 25, 2022
Relative to the previous attempt, this is rebased over the InstSimplify fix in ac74e7a, which addresses the miscompile reported in PR58401. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes llvm#57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
veselypeta
pushed a commit
to veselypeta/cherillvm
that referenced
this issue
May 22, 2024
foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes llvm/llvm-project#57448, which was my original motivation for the change.
veselypeta
pushed a commit
to veselypeta/cherillvm
that referenced
this issue
May 24, 2024
Reapply with a fix for the case where an operand simplified back to the original phi: We need to map this case to the new phi node. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes llvm/llvm-project#57448, which was my original motivation for the change.
veselypeta
pushed a commit
to veselypeta/cherillvm
that referenced
this issue
May 24, 2024
The infinite loop seen on buildbots should be fixed by 1189770 (assuming there are not multiple infinite combine loops...) ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes llvm/llvm-project#57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
veselypeta
pushed a commit
to veselypeta/cherillvm
that referenced
this issue
May 27, 2024
Relative to the previous attempt, this adjusts simplification to use the correct context instruction: We need to use the terminator of the incoming block, not the original instruction. ----- foldOpIntoPhi() currently only folds operations into the phi if all but one operands constant-fold. The two exceptions to this are freeze and select, where we allow more general simplification. This patch makes foldOpIntoPhi() generally simplification based and removes all the instruction-specific logic. We just try to simplify the instruction for each operand, and for the (potentially) one non-simplified operand, we move it into the new block with adjusted operands. This fixes llvm/llvm-project#57448, which was my original motivation for the change. Differential Revision: https://reviews.llvm.org/D134954
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This stays invariant under
-O2
. It should be possible to narrow%accum
toi1
, resulting in the following IR:The text was updated successfully, but these errors were encountered: