[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/fhahn approved this pull request. LGTM, thanks https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
@@ -177,16 +177,16 @@ define void @add_unique_ind32(ptr noalias nocapture %a,
i64 %n) {
; CHECK-LABEL: @add_unique_ind32(
; CHECK-NEXT: entry:
; CHECK-NEXT:[[TMP0:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:[[TMP1:%.*]] = shl i64 [[TMP0]], 2
+; CHECK-NEXT:[[TMP1:%.*]] = shl nuw i64 [[TMP0]], 2
fhahn wrote:
Ok great, this doesn't need to wait for the clarification in LangRef to land,
as long as it gets addressed soon.
https://github.com/llvm/llvm-project/pull/143532
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
@@ -177,16 +177,16 @@ define void @add_unique_ind32(ptr noalias nocapture %a,
i64 %n) {
; CHECK-LABEL: @add_unique_ind32(
; CHECK-NEXT: entry:
; CHECK-NEXT:[[TMP0:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:[[TMP1:%.*]] = shl i64 [[TMP0]], 2
+; CHECK-NEXT:[[TMP1:%.*]] = shl nuw i64 [[TMP0]], 2
fhahn wrote:
Yep the current users assume it doesn't wrap, but I think it would probably be
good to back that assumption with a adjustment to langref before relying even
more on that assumption. One way to do so would be to add a cap on the possible
value of `vscale`: https://github.com/llvm/llvm-project/pull/144607
https://github.com/llvm/llvm-project/pull/143532
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/david-arm approved this pull request. LGTM! I'm happy with this approach - we shouldn't really be calling CreateElementCount and CreateTypeSize with types that cannot fit the requested element count/type size, since that feels more like a bug with the caller. https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/paulwalker-arm edited https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
@@ -177,16 +177,16 @@ define void @add_unique_ind32(ptr noalias nocapture %a,
i64 %n) {
; CHECK-LABEL: @add_unique_ind32(
; CHECK-NEXT: entry:
; CHECK-NEXT:[[TMP0:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:[[TMP1:%.*]] = shl i64 [[TMP0]], 2
+; CHECK-NEXT:[[TMP1:%.*]] = shl nuw i64 [[TMP0]], 2
paulwalker-arm wrote:
This PR is not changing the definition of vscale. It only seeks to strengthen
the use of IRBuilder functions `CreateElementCount()` and `CreateTypeSize()`
where I'm wanting to mandate their specified result type must be big enough to
store the requested value, otherwise the result can be poison. My rational is
that it seems odd to use these functions and not care about generated the
correct value.
https://github.com/llvm/llvm-project/pull/143532
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/paulwalker-arm edited https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/paulwalker-arm edited https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
@@ -177,16 +177,16 @@ define void @add_unique_ind32(ptr noalias nocapture %a,
i64 %n) {
; CHECK-LABEL: @add_unique_ind32(
; CHECK-NEXT: entry:
; CHECK-NEXT:[[TMP0:%.*]] = call i64 @llvm.vscale.i64()
-; CHECK-NEXT:[[TMP1:%.*]] = shl i64 [[TMP0]], 2
+; CHECK-NEXT:[[TMP1:%.*]] = shl nuw i64 [[TMP0]], 2
fhahn wrote:
I guess the code here already implicitly assumes that the multiply does not
overflow, but it is not immedately clear how this follows from the definition
of `llvm.vscale` in LangRef, which says only limits `vscale` to a positive
constant:
```
vscale is a positive value 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 [poison
value](https://llvm.org/docs/LangRef.html#poisonvalues).
```
https://github.com/llvm/llvm-project/pull/143532
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LLVM][IRBuilder] Use NUW arithmetic for Create{ElementCount,TypeSize}. (PR #143532)
https://github.com/huntergr-arm approved this pull request. LGTM; I do think it's worth documenting the assumption/requirement about a sufficiently large type in the comments on the declarations for CreateElementCount and CreateTypeSize. https://github.com/llvm/llvm-project/pull/143532 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
