[all-commits] [llvm/llvm-project] e4fcbf: [X86] Pre-commit test case showing bug in combineO...

Björn Pettersson via All-commits all-commits at lists.llvm.org
Thu Sep 29 12:26:09 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e4fcbf3950092a78852e09f7cd8fe1d703967516
      https://github.com/llvm/llvm-project/commit/e4fcbf3950092a78852e09f7cd8fe1d703967516
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-09-29 (Thu, 29 Sep 2022)

  Changed paths:
    M llvm/test/CodeGen/X86/or-lea.ll

  Log Message:
  -----------
  [X86] Pre-commit test case showing bug in combineOr (X86ISelLowering.cpp)

In combineOr (X86ISelLowering.cpp) there is a DAG combine that rewrite
a "(0 - SetCC) | C" pattern into something simpler given that a LEA
can be used. Another requirement is that C has some specific value,
for example 1 or 7. When doing that check it is using a 32-bit
unsigned variable to store the value of C. So for a 64-bit OR this
could miscompile in case any of the 32 most significant bits in C
are set.

This patch adds a test case to show this miscompile bug.

Differential Revision: https://reviews.llvm.org/D134890


  Commit: 0513b0305acde91a0a5f7f5ea4061476011f0d1d
      https://github.com/llvm/llvm-project/commit/0513b0305acde91a0a5f7f5ea4061476011f0d1d
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-09-29 (Thu, 29 Sep 2022)

  Changed paths:
    M llvm/lib/Target/X86/X86ISelLowering.cpp
    M llvm/test/CodeGen/X86/or-lea.ll

  Log Message:
  -----------
  [X86] Avoid miscompile in combineOr (X86ISelLowering.cpp)

In combineOr (X86ISelLowering.cpp) there is a DAG combine that rewrite
a "(0 - SetCC) | C" pattern into something simpler given that a LEA
can be used. Another requirement is that C has some specific value,
for example 1 or 7. When checking those requirements the code used a
32-bit unsigned variable to store the value of C. So for a 64-bit OR
this could miscompile in case any of the 32 most significant bits in
C were non zero.

This patch adds fixes the bug by using a large enough type for the
C value.

The faulty code seem to have been introduced by commit 9bceb8981d32fe
(D131358).

Reviewed By: RKSimon

Differential Revision: https://reviews.llvm.org/D134892


Compare: https://github.com/llvm/llvm-project/compare/3a79f1caa9bd...0513b0305acd


More information about the All-commits mailing list