Missed optimization with `-fstrict-enums` for by-value parameters.

[keinflue]

In the following C++ code compiled with -O2 -fstrict-enums, the second function is optimized to return false unconditionally, while the first one isn't:

enum E { A, B };
bool f(E e) { return e > 1; };
bool g(E& e) { return e > 1; };

_Z1f1E:
        cmp     edi, 2
        setge   al
        ret

_Z1gR1E:
        xor     eax, eax
        ret

f should be optimized to return false unconditionally just like g.

The value range of E is [0,1] in C++ and it seems to me that this range information for the load of e is present in both cases coming from the Clang codegen, but is ignored and lost during the SROA pass:

define dso_local noundef zeroext i1 @_Z1f1E(i32 noundef %0) {
  %2 = alloca i32, align 4, !DIAssignID !23
  store i32 %0, ptr %2, align 4, !tbaa !25, !DIAssignID !29
  %3 = load i32, ptr %2, align 4, !tbaa !25, !range !31, !noundef !32
  %4 = icmp sgt i32 %3, 1
  ret i1 %4
}

define dso_local noundef zeroext i1 @_Z1gR1E(ptr noundef nonnull align 4 dereferenceable(4) %0) {
  %2 = alloca ptr, align 8, !DIAssignID !41
  store ptr %0, ptr %2, align 8, !tbaa !43, !DIAssignID !45
  %3 = load ptr, ptr %2, align 8, !tbaa !43, !nonnull !32, !align !47
  %4 = load i32, ptr %3, align 4, !tbaa !25, !range !31, !noundef !32
  %5 = icmp sgt i32 %4, 1
  ret i1 %5
}

;...
!31 = !{i32 0, i32 2}

=>

define dso_local noundef zeroext i1 @_Z1f1E(i32 noundef %0) {
  %2 = icmp sgt i32 %0, 1
  ret i1 %2
}

define dso_local noundef zeroext i1 @_Z1gR1E(ptr noundef nonnull align 4 dereferenceable(4) %0) {
  %2 = load i32, ptr %0, align 4, !tbaa !34, !range !38, !noundef !39
  %3 = icmp sgt i32 %2, 1
  ret i1 %3
}

;...
!38 = !{i32 0, i32 2}

https://godbolt.org/z/dz8xczYKP

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: None (keinflue)

In the following C++ code compiled with `-O2 -fstrict-enums`, the second function is optimized to return `false` unconditionally, while the first one isn't:

enum E { A, B };
bool f(E e) { return e > 1; };
bool g(E& e) { return e > 1; };

_Z1f1E:
        cmp     edi, 2
        setge   al
        ret

_Z1gR1E:
        xor     eax, eax
        ret

f should be optimized to return false unconditionally just like g.

The value range of E is [0,1] in C++ and it seems to me that this range information for the load of e is present in both cases coming from the Clang codegen, but is ignored and lost during the SROA pass:

define dso_local noundef zeroext i1 @<!-- -->_Z1f1E(i32 noundef %0) {
  %2 = alloca i32, align 4, !DIAssignID !23
  store i32 %0, ptr %2, align 4, !tbaa !25, !DIAssignID !29
  %3 = load i32, ptr %2, align 4, !tbaa !25, !range !31, !noundef !32
  %4 = icmp sgt i32 %3, 1
  ret i1 %4
}

define dso_local noundef zeroext i1 @<!-- -->_Z1gR1E(ptr noundef nonnull align 4 dereferenceable(4) %0) {
  %2 = alloca ptr, align 8, !DIAssignID !41
  store ptr %0, ptr %2, align 8, !tbaa !43, !DIAssignID !45
  %3 = load ptr, ptr %2, align 8, !tbaa !43, !nonnull !32, !align !47
  %4 = load i32, ptr %3, align 4, !tbaa !25, !range !31, !noundef !32
  %5 = icmp sgt i32 %4, 1
  ret i1 %5
}

;...
!31 = !{i32 0, i32 2}

=>

define dso_local noundef zeroext i1 @<!-- -->_Z1f1E(i32 noundef %0) {
  %2 = icmp sgt i32 %0, 1
  ret i1 %2
}

define dso_local noundef zeroext i1 @<!-- -->_Z1gR1E(ptr noundef nonnull align 4 dereferenceable(4) %0) {
  %2 = load i32, ptr %0, align 4, !tbaa !34, !range !38, !noundef !39
  %3 = icmp sgt i32 %2, 1
  ret i1 %3
}

;...
!38 = !{i32 0, i32 2}

https://godbolt.org/z/dz8xczYKP

[nikic]

Clang needs to emit range attributes.

[keinflue]

@nikic The range metadata is present in the original IR (unless I misunderstand something), see !range !31 on the loads in both functions.

But as an aside, this doesn't affect loads from bool or unsigned _BitInt(1) (in C23) which also have a restricted range relative to their underlying types because Clang doesn't only use the range metadata in this case but also emits a trunc/zext to retrieve the relevant bit(s). Maybe this should be done here as well?

[nikic]

@keinflue I'm referring to range attributes on the function argument, not the load metadata.

[keinflue]

I would like to write a patch for this, assuming there isn't a good reason not to add the range attribute for the parameter.

But I have two questions:

Firstly, I assume changing the parameter type to iN where N is the required number of bits is not valid for ABI reasons?

Secondly, while looking at the code I noticed that the function adding the range metadata to the load is also used for bool and types with bool representation:

bool f(bool* b) {
    return *b > 1;
}

define dso_local noundef zeroext i1 @_Z1fPb(ptr noundef %0) {
  %2 = alloca ptr, align 8
  store ptr %0, ptr %2, align 8, !tbaa !17
  %3 = load ptr, ptr %2, align 8, !tbaa !17
  %4 = load i8, ptr %3, align 1, !tbaa !25, !range !27, !noundef !28
  %5 = trunc i8 %4 to i1
  %6 = zext i1 %5 to i32
  %7 = icmp sgt i32 %6, 1
  ret i1 %7
}

;...
!27 = !{i8 0, i8 2}

Here range metadata is attached to the load while also extracting the relevant bits with trunc/zext. If I understood correctly, the range attribute on the load effectively makes it UB to load any value other than 0/1. trunc/zext is therefore redundant or is there a reason to have both that may also apply to enums?

[nikic]

Firstly, I assume changing the parameter type to iN where N is the required number of bits is not valid for ABI reasons?

Right. Maybe this is possible in some cases, but I wouldn't mess with it.

Secondly, while looking at the code I noticed that the function adding the range metadata to the load is also used for bool and types with bool representation:

bool f(bool* b) {
return *b > 1;
}

define dso_local noundef zeroext i1 @_Z1fPb(ptr noundef %0) {
%2 = alloca ptr, align 8
store ptr %0, ptr %2, align 8, !tbaa !17
%3 = load ptr, ptr %2, align 8, !tbaa !17
%4 = load i8, ptr %3, align 1, !tbaa !25, !range !27, !noundef !28
%5 = trunc i8 %4 to i1
%6 = zext i1 %5 to i32
%7 = icmp sgt i32 %6, 1
ret i1 %7
}

;...
!27 = !{i8 0, i8 2}

Here range metadata is attached to the load while also extracting the relevant bits with trunc/zext. If I understood correctly, the range attribute on the load effectively makes it UB to load any value other than 0/1. trunc/zext is therefore redundant or is there a reason to have both that may also apply to enums?

These trunc/zext aren't there to provide range information. They are just adjusting the bool between multiple representations. It's stored as i8 in memory, but represented as i1 in SSA, there is always a trunc on load. The zext here is to match the type between the bool and int used in the comparison.