[AMDGPU] Invalid integer comparison assertion hit

[jhuber6]

The following code hits an assertion, this case only appeared when I compiled with remarks enabled for some reason. https://godbolt.org/z/6v5s7aPs7. Without assertions it will successfully build.

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: Joseph Huber (jhuber6)

The following code hits an assertion, this case only appeared when I compiled with remarks enabled for some reason. https://godbolt.org/z/6v5s7aPs7. Without assertions it will successfully build.

[arsenm]

I got the test case a bit smaller, but it's still pretty big. There seem to be opportunities to shrink it by shuffling the order of functions around

target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
target triple = "amdgcn-amd-amdhsa"

define void @RunTic() {
  %call5.i1 = call i32 @G_CheckDemoStatus()
  tail call void @D_AdvanceDemo()
  call void @G_Ticker()
  ret void
}

define void @G_Ticker() {
  call void @G_DoReborn()
  tail call void @F_Ticker()
  tail call void @AM_Stop()
  tail call void @F_StartFinale()
  tail call void @D_AdvanceDemo()
  %call.i.i449 = call i32 @R_FlatNumForName()
  %call9.i.i = call i32 @R_TextureNumForName()
  %call.i306 = tail call ptr @P_TempSaveGameFile()
  %call1.i307 = call ptr @P_SaveGameFile()
  call void (...) @I_Error()
  ret void
}

define void @G_DoReborn() {
  call void @P_RemoveMobj()
  call void @P_SpawnMobj()
  call void @P_SpawnPlayer()
  call void (...) @I_Error()
  ret void
}

define void @AM_Stop() {
  ret void
}

define void @D_AdvanceDemo() {
  ret void
}

define void @F_StartFinale() {
  ret void
}

define void @F_Ticker() {
  ret void
}

define void @G_PlayerReborn() {
  ret void
}

define i32 @G_CheckDemoStatus() {
  tail call void @I_Quit()
  tail call void @D_AdvanceDemo()
  call void (...) @I_Error()
  ret i32 0
}

define void @HU_Start() {
  ret void
}

define void @I_Quit() {
  %fptr = load ptr, ptr null, align 8
  tail call void %fptr()
  ret void
}

define void @P_SetThingPosition() {
  ret void
}

define void @P_RemoveMobj() {
  ret void
}

define void @P_SpawnMobj() {
  ret void
}

define void @P_SpawnPlayer() {
  call void @G_PlayerReborn()
  call void @P_SetThingPosition()
  call void @P_SetupPsprites()
  tail call void @HU_Start()
  ret void
}

define void @P_SetupPsprites() {
  %fptr = load ptr, ptr addrspace(1) null, align 8
  tail call void %fptr()
  ret void
}

define ptr @P_TempSaveGameFile() {
  ret ptr null
}

define ptr @P_SaveGameFile() {
  ret ptr null
}

define i32 @R_TextureNumForName() {
  %ret = call i32 @R_FlatNumForName()
  ret i32 0
}

define void @I_Error(...) {
  %fptr = load ptr, ptr null, align 8
  call void %fptr()
  ret void
}

define i32 @R_FlatNumForName() {
  call void (...) @I_Error()
  unreachable
}

[JanekvO]

Doesn't seem to be too happy with the assignment at

llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp

Line 541 in 1e75d08

KBM[Expr] = KBM[Sym.getVariableValue(/*SetUsed=*/false)];

I may be missing some C++ knowledge but I assumed it'd call the default assignment operator for KnownBits which doesn't seem too complicated to copy/assign. Explicitly calling the defaulted copy constructor for the assign seems to do the trick but I'll look into reducing the test case further for a PR.

[JanekvO]

This case has been bothering me even though I knew the solution would be wrapping the DenseMap's return value using the KnownBits' copy constructor. Furthermore, most modifications to the test cases provided seem to sudden "make it work" again so I wasn't able to reduce the test cases as I'd like to so I went to look at what's going on internally.

The issue, for which I don't know the general solution to, seems to be that DenseMap's growth is invoked through KBM[Expr] assignment; however, KBM[SymVal] is retrieved prior to KBM[Expr] assignment so the APInt move constructor is called through DenseMap growth to move all old values to the new values which explicitly sets all old value BitWidths to 0. The KBM[Expr] is then set using the old retrieval of KBM[SymVal].