[PATCH] D114357: [CodeGen][AArch64] Ensure isSExtCheaperThanZExt returns true for negative constants

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D114357#3250516 <https://reviews.llvm.org/D114357#3250516>, @david-arm wrote:

> In D114357#3250506 <https://reviews.llvm.org/D114357#3250506>, @mstorsjo wrote:
>
>> I've bisected a miscompilation to this file.
>>
>> To reproduce:
>>
>>   $ git clone git://source.ffmpeg.org/ffmpeg
>>   $ cd ffmpeg
>>   $ ./configure --cc=clang --samples=$(pwd)/../samples
>>   $ make fate-rsync
>>   $ make -j$(nproc)
>>   $ make fate-dpcm-interplay
>>
>> The breakage happens in the libavformat/ipmovie.c file. (I also saw a couple other broken tests, so I think there are other source files affected too, but I didn't bisect and pinpoint those failures.)
>>
>> The issue can be observed with https://martin.st/temp/ipmovie-preproc.c, with `clang -target aarch64-linux-gnu -O3 -o - ipmovie-preproc.c`. The generated code contains differences like this:
>>
>>   --- old.s       2022-01-18 10:30:24.726016244 +0200
>>   +++ new.s       2022-01-18 10:30:01.650536299 +0200
>>   @@ -506,7 +506,7 @@
>>           mov     w1, #56
>>           bl      av_log
>>           add     x9, x19, #1104
>>   -       mov     w21, #65535
>>   +       mov     w21, #-1
>>    .LBB3_9:                                // %while.end
>>           ldr     x0, [x19]
>>           ldr     w8, [x0, #44]
>
> Hi @mstorsjo, thanks for the info. I'll revert the patch again for now. It's strange because ANY_EXTEND should really mean "any", i.e. you don't care if it's sign-extend or zero-extend! I suspect there is a bug in codegen somewhere that either relies upon ANY_EXTEND actually being ZERO_EXTEND, or relies upon ANY_EXTEND being consistently the same. I'm worried because the `isSExtCheaperThanZExt` interface definitely allows for the possibility of sometimes choosing one over the other depending upon the types.

I believe the culprit is FunctionLoweringInfo::ComputePHILiveOutRegInfo which assumes that ConstantInt inputs to phis will be zero extended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114357