[LLVM][LangRef] Restrict vscale to be a signed power-of-two integer.#183080
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-llvm-ir Author: Paul Walker (paulwalker-arm) ChangesThe rational for this change comes from #145098 and #183065 and essentially boils down to there being no known requirement to support non-power-of-two values of vscale and yet said support is leading to unnecessary complexity within LLVM. Hopefully it's ok to land the LangRef change in isolation and then follow up with the codebase simplifications. Full diff: https://github.com/llvm/llvm-project/pull/183080.diff 1 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 50a2515f69189..a7baeb94bc86b 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -4678,9 +4678,9 @@ elementtype may be any integer, floating-point, pointer type, or a sized
target extension type that has the ``CanBeVectorElement`` property. Vectors
of size zero are not allowed. For scalable vectors, the total number of
elements is a constant multiple (called vscale) of the specified number
-of elements; vscale is a positive integer that is unknown at compile time
-and the same hardware-dependent constant for all scalable vectors at run
-time. The size of a specific scalable vector type is thus constant within
+of elements; vscale is a positive power-of-two integer that is unknown at
+compile time and the same hardware-dependent constant for all scalable vectors
+at run time. The size of a specific scalable vector type is thus constant within
IR, even if the exact size in bytes cannot be determined until run time.
:Examples:
@@ -31501,8 +31501,8 @@ vectors such as ``<vscale x 16 x i8>``.
Semantics:
""""""""""
-``vscale`` is a positive value that is constant throughout program
-execution, but is unknown at compile time.
+``vscale`` is a positive power-of-two integer that is constant throughout
+program execution, but is unknown at compile time.
If the result value does not fit in the result type, then the result is
a :ref:`poison value <poisonvalues>`.
|
nikic
left a comment
There was a problem hiding this comment.
LGTM, based on past discussions there is broad consensus in favor of this change.
lukel97
left a comment
There was a problem hiding this comment.
LGTM, thanks. +1 on splitting off the removal of the TTI hook etc
|
MLIR (when lowering to LLVM) inherits |
Thanks @MacDue. I can see a reference in Dialects/Vector.md that I'll need to update. |
After llvm#183080 vscale can no longer be a power of 2, which means the canonical IV can't overflow with tail folding w/ scalable vectors anymore. Therefore we don't need to drop the NUW flag. IVUpdateMayOverflow is left to be removed in a separate PR since it removes further runtime checks.
After #183080 this is no longer a configurable property. NOTE: No test changes expected beyond llvm/test/Transforms/LoopVectorize/scalable-predication.ll which has been removed because it only existed to verfiy the now unsupported functionality.
…(#183292) After llvm/llvm-project#183080 this is no longer a configurable property. NOTE: No test changes expected beyond llvm/test/Transforms/LoopVectorize/scalable-predication.ll which has been removed because it only existed to verfiy the now unsupported functionality.
… NFC The IV can no longer overflow with tail folding after llvm#183080.
…lvm#183080) There is no known requirement to support non-power-of-two values of vscale and yet said support is leading to unnecessary complexity within LLVM.
Previously vscale_range used to add the constraint that vscale is a power-of-two, but after llvm#183080 it's already a power-of-two to begin with. This drops the extra mention of power-of-two to clarify it's not a new constraint now.
After llvm#183080 vscale is always a power of two, and we don't need to check for the vscale_range attribute.
After #183080 vscale can no longer be a non-power of 2, which means the canonical IV can't overflow with tail folding w/ scalable vectors anymore. Therefore we don't need to drop the NUW flag. IVUpdateMayOverflow is left to be removed in a separate PR since it removes further runtime checks.
After llvm#183080, the canonical IV (not the increment!) can't overflow. So now canonical IVs that are unrolled will have steps that don't overflow, so we can add the nuw flag. This allows us to tighten the VPlanVerifier isKnownMonotonic check by restricting it to adds with nuw.
No longer holds after llvm#183080
After #183080, the canonical IV (not the increment!) can't overflow. So now canonical IVs that are unrolled will have steps that don't overflow, so we can add the nuw flag. This allows us to tighten the VPlanVerifier isKnownMonotonic check by restricting it to adds with nuw.
After llvm#183080 this is no longer a configurable property. NOTE: No test changes expected beyond llvm/test/Transforms/LoopVectorize/scalable-predication.ll which has been removed because it only existed to verfiy the now unsupported functionality.
… NFC (llvm#183066) The IV can no longer overflow with tail folding after llvm#183080.
After llvm#183080 vscale can no longer be a non-power of 2, which means the canonical IV can't overflow with tail folding w/ scalable vectors anymore. Therefore we don't need to drop the NUW flag. IVUpdateMayOverflow is left to be removed in a separate PR since it removes further runtime checks.
…lvm#183693) After llvm#183080 vscale is always a power of two, so we don't need to check for the vscale_range attribute.
No longer holds after llvm#183080
…ut the attribute (llvm#183689) Previously vscale_range used to add the constraint that vscale is a power-of-two, but after llvm#183080 it's already a power-of-two to begin with. This clarifies the sentence about assumptions when there is no attribute
After llvm#183080, the canonical IV (not the increment!) can't overflow. So now canonical IVs that are unrolled will have steps that don't overflow, so we can add the nuw flag. This allows us to tighten the VPlanVerifier isKnownMonotonic check by restricting it to adds with nuw.
…lvm#183080) There is no known requirement to support non-power-of-two values of vscale and yet said support is leading to unnecessary complexity within LLVM.
The rational for this change comes from #145098 and #183065 and essentially boils down to there being no known requirement to support non-power-of-two values of vscale and yet said support is leading to unnecessary complexity within LLVM.
Hopefully it's ok to land the LangRef change in isolation and then follow up with the codebase simplifications.