[all-commits] [llvm/llvm-project] 115cb4: [WebAssembly] Don't fold non-nuw add/sub in FastIS...

[Heejin Ahn via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 115cb402d8ed91f94d22afcc4c2c9ed9def53cc7
      https://github.com/llvm/llvm-project/commit/115cb402d8ed91f94d22afcc4c2c9ed9def53cc7
  Author: Heejin Ahn <aheejin at gmail.com>
  Date:   2024-10-09 (Wed, 09 Oct 2024)

  Changed paths:
    M llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
    A llvm/test/CodeGen/WebAssembly/fast-isel-no-offset.ll
    M llvm/test/CodeGen/WebAssembly/fast-isel-pr47040.ll

  Log Message:
  -----------
  [WebAssembly] Don't fold non-nuw add/sub in FastISel (#111278)

We should not fold one of add/sub operands into a load/store's offset
when `nuw` (no unsigned wrap) is not present, because the address
calculation, which adds the offset with the operand, does not wrap.

This is handled correctly in the normal ISel:

https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp#L328-L332
but not in FastISel.

This positivity check in FastISel is not sufficient to avoid this case
fully:

https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L348-L352

because
1. Even if RHS is within signed int range, depending on the value of the
LHS, the resulting value can exceed uint32 max.
2. When one of the operands is a label, `Address` can contain a
`GlobalValue` and a `Reg` at the same time, so the `GlobalValue` becomes
incorrectly an offset:
https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L53-L69
https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp#L409-L417

Both cases are in the newly added test.

We should handle `SUB` too because `SUB` is the same as `ADD` when RHS's
sign changes. I checked why our current normal ISel only handles `ADD`,
and the reason it's OK for the normal ISel to handle only `ADD` seems
that DAGCombiner replaces `SUB` with `ADD` here:
https://github.com/llvm/llvm-project/blob/6de5305b3d7a4a19a29b35d481a8090e2a6d3a7e/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L3904-L3907

Fixes #111018.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications