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(-)
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;
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-n....
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/3a997f4ee640e607a171a196....
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 ---
v1 -> v2:
* Remove shadow altogether, as suggested by Joe Perches. * Add Nick's Reviewed-by, as I assume it still stands.
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)
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
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
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
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Joe Perches -
Nathan Chancellor -
Nick Desaulniers