| Message ID | 20200318002500.52471-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdgpu: Initialize shadow to false in gfx_v9_0_rlcg_wreg | expand |
On Tue, Mar 17, 2020 at 5:25 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > > clang warns: > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow' > is used uninitialized whenever 'if' condition is > false [-Wsometimes-uninitialized] > if (offset == grbm_cntl || offset == grbm_idx) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use > occurs here > if (shadow) { > ^~~~~~ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:2: note: remove the 'if' if > its condition is always true > if (offset == grbm_cntl || offset == grbm_idx) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:738:13: note: initialize the > variable 'shadow' to silence this warning > bool shadow; > ^ > = 0 > 1 warning generated. > > It is not wrong so initialize shadow to false to ensure shadow is always > used initialized. Yep, thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Fixes: 2e0cc4d48b91 ("drm/amdgpu: revise RLCG access path") > Link: https://github.com/ClangBuiltLinux/linux/issues/936 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 7bc2486167e7..affbff76758c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -735,7 +735,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) > static void *spare_int; > static uint32_t grbm_cntl; > static uint32_t grbm_idx; > - bool shadow; > + bool shadow = false; > > scratch_reg0 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4; > scratch_reg1 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4; > -- > 2.26.0.rc1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200318002500.52471-1-natechancellor%40gmail.com.
On Tue, 2020-03-17 at 17:25 -0700, Nathan Chancellor wrote: > clang warns: > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow' > is used uninitialized whenever 'if' condition is > false [-Wsometimes-uninitialized] > if (offset == grbm_cntl || offset == grbm_idx) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use > occurs here > if (shadow) { > ^~~~~~ Wouldn't it be better to get rid of the shadow variable completely? --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 7bc248..496b9e 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -735,7 +735,6 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) static void *spare_int; static uint32_t grbm_cntl; static uint32_t grbm_idx; - bool shadow; scratch_reg0 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4; scratch_reg1 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4; @@ -751,10 +750,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) return; } - if (offset == grbm_cntl || offset == grbm_idx) - shadow = true; - - if (shadow) { + if (offset == grbm_cntl || offset == grbm_idx) { if (offset == grbm_cntl) writel(v, scratch_reg2); else if (offset == grbm_idx)
On Wed, Mar 18, 2020 at 1:28 PM Joe Perches <joe@perches.com> wrote: > > On Tue, 2020-03-17 at 17:25 -0700, Nathan Chancellor wrote: > > clang warns: > > > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow' > > is used uninitialized whenever 'if' condition is > > false [-Wsometimes-uninitialized] > > if (offset == grbm_cntl || offset == grbm_idx) > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use > > occurs here > > if (shadow) { > > ^~~~~~ > > Wouldn't it be better to get rid of the shadow variable completely? Yes, much better indeed. Seems it only has one use. > --- > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > index 7bc248..496b9e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > @@ -735,7 +735,6 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) > static void *spare_int; > static uint32_t grbm_cntl; > static uint32_t grbm_idx; > - bool shadow; > > scratch_reg0 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4; > scratch_reg1 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4; > @@ -751,10 +750,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) > return; > } > > - if (offset == grbm_cntl || offset == grbm_idx) > - shadow = true; > - > - if (shadow) { > + if (offset == grbm_cntl || offset == grbm_idx) { > if (offset == grbm_cntl) > writel(v, scratch_reg2); > else if (offset == grbm_idx) > > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/3a997f4ee640e607a171a19668f5f5484062116c.camel%40perches.com.
On Wed, Mar 18, 2020 at 5:08 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Mar 18, 2020 at 2:05 PM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > clang warns: > > > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow' > > is used uninitialized whenever 'if' condition is > > false [-Wsometimes-uninitialized] > > if (offset == grbm_cntl || offset == grbm_idx) > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use > > occurs here > > if (shadow) { > > ^~~~~~ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:2: note: remove the 'if' if > > its condition is always true > > if (offset == grbm_cntl || offset == grbm_idx) > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:738:13: note: initialize the > > variable 'shadow' to silence this warning > > bool shadow; > > ^ > > = 0 > > 1 warning generated. > > > > shadow is only assigned in one condition and used as the condition for > > another if statement; combine the two if statements and remove shadow > > to make the code cleaner and resolve this warning. > > > > Fixes: 2e0cc4d48b91 ("drm/amdgpu: revise RLCG access path") > > Link: https://github.com/ClangBuiltLinux/linux/issues/936 > > Suggested-by: Joe Perches <joe@perches.com> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Applied. thanks! Alex > > --- > > > > v1 -> v2: > > > > * Remove shadow altogether, as suggested by Joe Perches. > > * Add Nick's Reviewed-by, as I assume it still stands. > > yep, thanks > > > > > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > index 7bc2486167e7..496b9edca3c3 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c > > @@ -735,7 +735,6 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) > > static void *spare_int; > > static uint32_t grbm_cntl; > > static uint32_t grbm_idx; > > - bool shadow; > > > > scratch_reg0 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4; > > scratch_reg1 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4; > > @@ -751,10 +750,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) > > return; > > } > > > > - if (offset == grbm_cntl || offset == grbm_idx) > > - shadow = true; > > - > > - if (shadow) { > > + if (offset == grbm_cntl || offset == grbm_idx) { > > if (offset == grbm_cntl) > > writel(v, scratch_reg2); > > else if (offset == grbm_idx) > > -- > > 2.26.0.rc1 > > > > > -- > Thanks, > ~Nick Desaulniers > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c index 7bc2486167e7..affbff76758c 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -735,7 +735,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v) static void *spare_int; static uint32_t grbm_cntl; static uint32_t grbm_idx; - bool shadow; + bool shadow = false; scratch_reg0 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4; scratch_reg1 = adev->rmmio + (adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4;
clang warns: drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (offset == grbm_cntl || offset == grbm_idx) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use occurs here if (shadow) { ^~~~~~ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:2: note: remove the 'if' if its condition is always true if (offset == grbm_cntl || offset == grbm_idx) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:738:13: note: initialize the variable 'shadow' to silence this warning bool shadow; ^ = 0 1 warning generated. It is not wrong so initialize shadow to false to ensure shadow is always used initialized. Fixes: 2e0cc4d48b91 ("drm/amdgpu: revise RLCG access path") Link: https://github.com/ClangBuiltLinux/linux/issues/936 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)