[all-commits] [llvm/llvm-project] c2a879: [VPlan] Fix VPTypeAnalysis cache clobbering in EVL...

[Luke Lau via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: c2a879ecaa71cdff35b10bd656f6781e808bdec8
      https://github.com/llvm/llvm-project/commit/c2a879ecaa71cdff35b10bd656f6781e808bdec8
  Author: Luke Lau <luke at igalia.com>
  Date:   2024-12-18 (Wed, 18 Dec 2024)

  Changed paths:
    M llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
    A llvm/test/Transforms/LoopVectorize/RISCV/type-info-cache-evl-crash.ll

  Log Message:
  -----------
  [VPlan] Fix VPTypeAnalysis cache clobbering in EVL transform (#120252)

When building SPEC CPU 2017 with RISC-V and EVL tail folding, this
assertion in VPTypeAnalysis would trigger during the transformation to
EVL recipes:


https://github.com/llvm/llvm-project/blob/d8a0709b1090350a7fe3604d8ab78c7d62f10698/llvm/lib/Transforms/Vectorize/VPlanAnalysis.cpp#L135-L142

It was caused by this recipe:

```
WIDEN ir<%shr> = vp.or ir<%add33>, ir<0>, vp<%6>
```

Having its type inferred as i16, when ir<%add33> and ir<0> had inferred
types of i32 somehow.

The cause of this turned out to be because the VPTypeAnalysis cache was
getting clobbered: In this transform we were erasing recipes but keeping
around the same mapping from VPValue* to Type*. In the meantime, new
recipes would be created which would have the same address as the old
value. They would then incorrectly get the old erased VPValue*'s cached
type:

```
  --- before ---
  0x600001ec5030: WIDEN ir<%mul21.neg> = vp.mul vp<%11>, ir<0>, vp<%6>
  0x600001ec5450: <badref> <- some value that was erased
  --- after ---
  0x600001ec5030: WIDEN ir<%mul21.neg> = vp.mul vp<%11>, ir<0>, vp<%6>
  0x600001ec5450: WIDEN ir<%shr> = vp.or ir<%add33>, ir<0>, vp<%6>  <- a new value that happens to have the same address
```

This fixes this by deferring the erasing of recipes till after the
transformation.

The test case might be a bit flakey since it just happens to have the
right conditions to recreate this. I tried to add an assert in
inferScalarType that every VPValue in the cache was valid, but couldn't
find a way of telling if a VPValue had been erased.

---------

Co-authored-by: Florian Hahn <flo at fhahn.com>



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