[llvm] [Float2Int] Resolve FIXME: Pick the smallest legal type that fits (PR #79158)

Jessica Clarke via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 19 17:07:29 PDT 2024


jrtc27 wrote:

> <img alt="Screenshot 2024-03-19 at 3 31 09 PM" width="668" src="https://private-user-images.githubusercontent.com/83477269/314202929-24bbe8ef-c227-4f0b-b820-955e400e030a.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTA4OTMxNDEsIm5iZiI6MTcxMDg5Mjg0MSwicGF0aCI6Ii84MzQ3NzI2OS8zMTQyMDI5MjktMjRiYmU4ZWYtYzIyNy00ZjBiLWI4MjAtOTU1ZTQwMGUwMzBhLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAzMjAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMzIwVDAwMDA0MVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWI4OWUwZjk5ZWNhZTgwOGQwMmFiZmI5ZDcxMTdiZDExNzY3N2FkNDcyNTQ4NmFjN2ZhNThlMTljZjc1NzExZjUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.WBOZak3o9iz9WaMkHHz_dhBQr2h3TvsUaZeeNTb2mmw"> The issue in question is not the Float2Int pass.

Well most passes are pretty uninteresting if you run them on pre-SROA/mem2reg IR. Looking at the later godbolt link, there's the following transform for Float2IntPass:

Before:
```
define dso_local noundef i32 @main() local_unnamed_addr {
entry:
  %x = alloca i32, align 4
    #dbg_assign(i1 undef, !18, !DIExpression(), !21, ptr %x, !DIExpression(), !22)
  call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %x)
  store volatile i32 1, ptr %x, align 4
    #dbg_assign(i32 1, !18, !DIExpression(), !29, ptr %x, !DIExpression(), !22)
  %x.0.x.0.x.0. = load volatile i32, ptr %x, align 4
  %cmp = icmp sgt i32 %x.0.x.0.x.0., 0
    #dbg_value(i1 %cmp, !20, !DIExpression(DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value), !22)
  %conv = uitofp i1 %cmp to float
  %mul = fmul float %conv, 2.550000e+02
  %conv1 = fptoui float %mul to i8
  %conv2 = zext i8 %conv1 to i32
  call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %x)
  ret i32 %conv2
}
```

After:
```
define dso_local noundef i32 @main() local_unnamed_addr {
entry:
  %x = alloca i32, align 4
    #dbg_assign(i1 undef, !18, !DIExpression(), !21, ptr %x, !DIExpression(), !22)
  call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %x)
  store volatile i32 1, ptr %x, align 4
    #dbg_assign(i32 1, !18, !DIExpression(), !29, ptr %x, !DIExpression(), !22)
  %x.0.x.0.x.0. = load volatile i32, ptr %x, align 4
  %cmp = icmp sgt i32 %x.0.x.0.x.0., 0
    #dbg_value(i1 %cmp, !20, !DIExpression(DW_OP_LLVM_convert, 1, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value), !22)
  %0 = zext i1 %cmp to i8
  %mul3 = mul i8 %0, 127
  %conv2 = zext i8 %mul3 to i32
  call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %x)
  ret i32 %conv2
}
```

This is why you've received feedback about these kinds of small changes being awkward to review. When they don't offer significant gains, they are often outweighed by the high risk that something will break, whether that's because the change in question is incorrect or because it exposes incorrect code elsewhere.

https://github.com/llvm/llvm-project/pull/79158


More information about the llvm-commits mailing list