[PATCH] D88684: llvm-reduce: Don't replace intrinsic calls with undef

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

>> In D88684#2327660 <https://reviews.llvm.org/D88684#2327660>, @arsenm wrote:
>>
>>> In D88684#2318429 <https://reviews.llvm.org/D88684#2318429>, @dblaikie wrote:
>>>
>>>> In D88684#2318361 <https://reviews.llvm.org/D88684#2318361>, @arsenm wrote:
>>>>
>>>>> In D88684#2318108 <https://reviews.llvm.org/D88684#2318108>, @ychen wrote:
>>>>>
>>>>>> The test tieing to AMDGPU seems less ideal for general functionality. I've seen llvm.dbg.value caused the same problem, maybe we could use that?
>>>>>
>>>>> The debug intrinsics require a *lot* of metadata overhead simply to satsify the operations. The other metadata using intrinsics are experimental, so this seemed like the least fragile way to test it
>>>>
>>>> Given the functionality isn't implemented in terms of metadata-using-intrinsics in particular, a test using some non-metadata-using intrinsic would be fine.
>>>
>>> I'm specifically checking for the invalid intrinsic error produced. I think another intrinsic would be less clear for what is actually happening
>>
>> Ah, please tweak the test to test the general behavior (not modifying intrinsic calls), not the invalidity/second order that lead to the root cause/change in behavior (that some of those modified intrinsic calls then become invalid).
>
> I don't see how else I would tell from the output that the intrinsic call itself is why it was skipped. The messages printed don't directly indicate why anything was skipped. The interestingness script needs to keep the call around

Sorry, not sure I follow - You have two calls, the difference is one is an intrinsic and one is not and the reduction continues until there's only one call & then the final check ensures the remaining call is the intrinsic you're looking for.

For instance, this modification of the test seems to fail without the patch and pass with it, but doesn't rely on a target-specific intrinsic or metadata parameters:

  ; RUN: llvm-reduce --test FileCheck --test-arg --check-prefixes=ALL,CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t 2> %t.log
  ; RUN: cat %t | FileCheck -implicit-check-not=uninteresting --check-prefixes=ALL,CHECK-FINAL %s
  
  ; Check that the call is removed by instruction reduction passes
  ; RUN: llvm-reduce --test FileCheck --test-arg --check-prefixes=ALL,CHECK-FUNC --test-arg %s --test-arg --input-file %s -o %t
  ; RUN: cat %t | FileCheck -implicit-check-not=uninteresting --check-prefixes=ALL,CHECK-NOCALL %s
  
  
  declare i8* @llvm.sponentry.p0i8()
  declare void @uninteresting()
  
  ; ALL-LABEL: define i8* @interesting(
  define i8* @interesting() {
  entry:
    ; CHECK-INTERESTINGNESS: call
    ; CHECK-NOCALL-NOT: call
  
    ; CHECK-FINAL: %call = call i8* @llvm.sponentry.p0i8()
    ; CHECK-FINAL-NEXT: ret i8* %call
    %call = call i8* @llvm.sponentry.p0i8()
    call void @uninteresting()
    ret i8* %call
  }

Might be able to be simplified further - test seems to work even with a void return from @interesting and the "CHECK-FUNC" on the second reduce run doesn't appear to be used (there are no "CHECK-FUNC" check lines).

Could you check in something like this, a simplified/reduced version of the test?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88684/new/

https://reviews.llvm.org/D88684