[all-commits] [llvm/llvm-project] 3f3018: [SimplifyLibCalls] Pre-commit test case showing bu...

[Björn Pettersson via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 3f3018b602c2650358c9ff26657b2d8ba7da7845
      https://github.com/llvm/llvm-project/commit/3f3018b602c2650358c9ff26657b2d8ba7da7845
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-10-07 (Fri, 07 Oct 2022)

  Changed paths:
    M llvm/test/Transforms/InstCombine/wcslen-1.ll

  Log Message:
  -----------
  [SimplifyLibCalls] Pre-commit test case showing bug with wide char support

The ValueTracking support for getting the string length of a wchar_t
string (e.g. using wcslen) seem to be having some bugs.

Problem I've seen is that llvm::getConstantDataArrayInfo is taking
both a "ElementSize" argument (basically indicating size of a
char/element in bits) and an "Offset" which afaict is an offset
in the unit "number of elements". Then it also use
stripAndAccumulateConstantOffsets to get a "StartIdx" which afaict
is calculated in bytes. The returned Slice.Length is based on
arithmetics that add/subtract variables that are having different
units (bytes vs elements). Most notably I think the "StartIdx" must
be scaled using the "ElementSize" to get correct results.

This patch just adds a new test case showing that we get a wrong
result when doing wcslen(x + c). The actual fix to the above problem
will be done in a follow up commit.

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


  Commit: 01e1f3297151231fbd73705a073f42f2c453855d
      https://github.com/llvm/llvm-project/commit/01e1f3297151231fbd73705a073f42f2c453855d
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-10-07 (Fri, 07 Oct 2022)

  Changed paths:
    M llvm/lib/Analysis/ValueTracking.cpp
    M llvm/test/Transforms/InstCombine/wcslen-1.ll

  Log Message:
  -----------
  [ValueTracking][SimplifyLibCalls] Fix bug in getConstantDataArrayInfo for wchar_t

When SimplifyLibCalls is dealing with wchar_t (e.g. optimizing wcslen)
it uses ValueTracking helpers with a CharSize/ElementSize that isn't
8, but rather 16 or 32 (to match with the size in bits of a wchar_t).

Problem I've seen is that llvm::getConstantDataArrayInfo is taking
both an "ElementSize" argument (basically indicating size of a
char/element in bits) and an "Offset" which afaict is an offset
in the unit "number of elements". Then it also use
stripAndAccumulateConstantOffsets to get a "StartIdx" which afaict
is calculated in bytes. The returned Slice.Length is based on
arithmetics that add/subtract variables that are having different
units (bytes vs elements). Most notably I think the "StartIdx" must
be scaled using the "ElementSize" to get correct results.

The symptom of the above problem was seen in the wcslen-1.ll test
case which miscompiled.

This patch is supposed to resolve the bug by converting between
bytes and elements when needed.

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


  Commit: 8ee529a39895c7b1c54b91c71815df077adb21dc
      https://github.com/llvm/llvm-project/commit/8ee529a39895c7b1c54b91c71815df077adb21dc
  Author: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
  Date:   2022-10-07 (Fri, 07 Oct 2022)

  Changed paths:
    M llvm/test/Transforms/ExpandMemCmp/X86/bcmp.ll
    M llvm/test/Transforms/ExpandMemCmp/X86/memcmp-x32.ll
    M llvm/test/Transforms/ExpandMemCmp/X86/memcmp.ll

  Log Message:
  -----------
  [test][ExpandMemCmp] Convert test cases to opaque pointers. NFC

Conversion performed using the script at:
https://gist.github.com/nikic/98357b71fd67756b0f064c9517b62a34


Compare: https://github.com/llvm/llvm-project/compare/8f8e4e3b38c4...8ee529a39895