Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp:225: suspicious test ? #40458

Closed Quuxplusone closed 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR41488
Status RESOLVED FIXED
Importance P normal
Reported by David Binderman (dcb314@hotmail.com)
Reported on 2019-04-13 05:00:57 -0700
Last modified on 2019-04-15 03:37:17 -0700
Version trunk
Hardware PC Linux
CC htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, tpr.ll@botech.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
llvm/lib/Target/AMDGPU/SIOptimizeExecMaskingPreRA.cpp:225]: (style) Same
expression on both sides of '||'.

Source code is

  if (TII->hasModifiersSet(*Sel, AMDGPU::OpName::src0_modifiers) ||
      TII->hasModifiersSet(*Sel, AMDGPU::OpName::src0_modifiers))

Maybe better code

  if (TII->hasModifiersSet(*Sel, AMDGPU::OpName::src0_modifiers) ||
      TII->hasModifiersSet(*Sel, AMDGPU::OpName::src1_modifiers))
Quuxplusone commented 5 years ago

Thanks David, I think you're right. The source modifiers tested there are currently never set in codegen, so the bug won't show up, but it could show up if we start setting source modifiers on v_cndmask instructions in the future.

Quuxplusone commented 5 years ago

D60652

Quuxplusone commented 5 years ago

Fixed rL358392