[PATCH] D155051: [AMD] ]Add sanity check that fixes bad shift operation in AMD backend

Konrad Kusiak via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 12 01:57:55 PDT 2023


konradkusiak97 created this revision.
konradkusiak97 added a reviewer: foad.
Herald added subscribers: StephenFan, kerbowa, hiraditya, jvesely, arsenm.
Herald added a project: All.
konradkusiak97 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

There is a problem with the `SILoadStoreOptimizer::dmasksCanBeCombined()` function that can lead to UB.

This boolean function decides if two masks can be combined into 1. The idea here is that the bits which are "on" in one mask, don't overlap with the "on" bits of the other. Consider an example (10 bits for simplicity):

Mask 1: `0101101000`
Mask 2: `0000000110`

Those can be combined into a single mask: `0101101110`.

To check if such an operation is possible, the code takes the mask which is greater and counts how many `0`s there are, starting from the `LSB` and stopping at the first `1`. Then, it shifts `1u` by this number and compares it with the smaller mask. The problem is that when both masks are `0`, the counter will find `32` zeroes in the first mask and will try to do a shift by `32` positions which leads to UB.

The fix is a simple sanity check, if the bigger mask is 0 or not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155051

Files:
  llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp


Index: llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
+++ llvm/lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
@@ -871,6 +871,10 @@
   unsigned MaxMask = std::max(CI.DMask, Paired.DMask);
   unsigned MinMask = std::min(CI.DMask, Paired.DMask);
 
+  // Sanity check - if both masks are 0
+  if (!MaxMask) 
+    return true;
+
   unsigned AllowedBitsForMin = llvm::countr_zero(MaxMask);
   if ((1u << AllowedBitsForMin) <= MinMask)
     return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155051.539424.patch
Type: text/x-patch
Size: 565 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230712/ca1edb51/attachment.bin>


More information about the llvm-commits mailing list