Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Isel crash with large constant (csmith) #33832

Closed Quuxplusone closed 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR34859
Status RESOLVED FIXED
Importance P enhancement
Reported by Jonas Paulsson (paulsson@linux.vnet.ibm.com)
Reported on 2017-10-06 00:56:45 -0700
Last modified on 2017-11-14 12:00:52 -0800
Version trunk
Hardware PC Linux
CC llvm-bugs@lists.llvm.org, paulsson@linux.vnet.ibm.com, uweigand@de.ibm.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
This (reduced) program:

@g_272 = external global i64, align 8
@g_276 = external global i16, align 2

; Function Attrs: noreturn nounwind
define void @main(i32 signext) local_unnamed_addr #0 {
  store i16 -1, i16* @g_276, align 2
  %2 = load i16, i16* @g_276, align 2
  %3 = icmp ne i16 %2, 0
  %4 = zext i1 %3 to i64
  %5 = or i64 %4, 3944173009226982604
  store i64 %5, i64* @g_272, align 8
  tail call void @transparent_crc(i64 3944173009226982604, i32 signext undef)
  unreachable
}

declare void @transparent_crc(i64, i32 signext) local_unnamed_addr #1

+
bin/llc -mtriple=s390x-linux-gnu -mcpu=z13

crashes with

LLVM ERROR: Cannot select: t47: i64 = Constant<3944173007420784641>

DAG:

Optimized legalized selection DAG: BB#0 'main:'
SelectionDAG has 25 nodes:
  t0: ch = EntryToken
          t45: i64 = SystemZISD::PCREL_WRAPPER TargetGlobalAddress:i64<i16* @g_276> 0
        t41: ch = store<ST2[@g_276], trunc to i16> t0, Constant:i32<65535>, t45, undef:i64
          t18: i64 = or Constant:i64<1>, OpaqueConstant:i64<3944173009226982604>
          t43: i64 = SystemZISD::PCREL_WRAPPER TargetGlobalAddress:i64<i64* @g_272> 0
        t34: ch = store<ST8[@g_272]> t0, t18, t43, undef:i64
      t35: ch = TokenFactor t41, t34
    t24: ch,glue = callseq_start t35, TargetConstant:i64<0>, TargetConstant:i64<0>
  t28: ch,glue = CopyToReg t24, Register:i64 %R2D, OpaqueConstant:i64<3944173009226982604>
  t30: ch,glue = CopyToReg t28, Register:i64 %R3D, Constant:i64<0>, t28:1
    t26: i64 = SystemZISD::PCREL_WRAPPER TargetGlobalAddress:i64<void (i64, i32)* @transparent_crc> 0
  t32: ch,glue = SystemZISD::CALL t30, t26, Register:i64 %R2D, Register:i64 %R3D, RegisterMask:Untyped, t30:1
  t33: ch,glue = callseq_end t32, TargetConstant:i64<0>, TargetConstant:i64<0>, t32:1
Quuxplusone commented 6 years ago
Note : the commits for this were:
LLVM : e5e1c85 / trunk@314416
clang: 0e1f5c3 / cfe/trunk@314391
Quuxplusone commented 6 years ago
Initial selection DAG: BB#0 'main:'
...
           t16: i1 = setcc t13, Constant:i16<0>, setne:ch
          t17: i64 = zero_extend t16
        t18: i64 = or t17, OpaqueConstant:i64<3944173009226982604>
...

=>

Optimized lowered selection DAG: BB#0 'main:'
...
        t18: i64 = or Constant:i64<1>, OpaqueConstant:i64<3944173009226982604>
...

I suspect that the SystemZ backend (splitLargeImmediate() is not expecting to
handle an or with two constants?

I am also guessing that the Opaque bit is somehow preventing the DAGCombiner to
constant fold t18. Not sure exactly why...
Quuxplusone commented 6 years ago
(In reply to Jonas Paulsson from comment #2)
> Initial selection DAG: BB#0 'main:'
> ...
>            t16: i1 = setcc t13, Constant:i16<0>, setne:ch
>           t17: i64 = zero_extend t16
>         t18: i64 = or t17, OpaqueConstant:i64<3944173009226982604>
> ...
>
> =>
>
> Optimized lowered selection DAG: BB#0 'main:'
> ...
>         t18: i64 = or Constant:i64<1>,
> OpaqueConstant:i64<3944173009226982604>
> ...
>
>
> I suspect that the SystemZ backend (splitLargeImmediate() is not expecting
> to handle an or with two constants?
>
> I am also guessing that the Opaque bit is somehow preventing the DAGCombiner
> to constant fold t18. Not sure exactly why...

I believe this is deliberate, since the large constant is reused a second time,
so it may be better to only materialize the large constant once, and then
actually perform an or with 1 to get the second constant, rather than having to
materialize two different large constants.

The back-end should simply leave the OR alone in this case.  I'm testing a
patch ...
Quuxplusone commented 6 years ago

Should be fixed by r318187.