-Wmaybe-uninitialized #428
Conversation
| const N* n; | ||
| if (isXtn2) { | ||
| d = sourceValues[0].getAsVector<D>(); | ||
| n = sourceValues[1].getAsVector<N>(); | ||
| } else { | ||
| d = {}; | ||
| n = sourceValues[0].getAsVector<N>(); | ||
| } | ||
|
|
||
| D out[16 / sizeof(D)] = {0}; | ||
| int index = 0; | ||
|
|
||
| for (int i = 0; i < I; i++) { | ||
| if (isXtn2 & (i < (I / 2))) { | ||
| assert(isXtn2 && "isXtn2 is false so d is not initialised"); | ||
| out[i] = d[i]; | ||
| } else { | ||
| out[i] = static_cast<D>(n[index]); |
There was a problem hiding this comment.
@FinnWilkinson this is a temporary solution as I didn't want to fiddle too much with the for loop below. If you have a preferred solution then feel free to push a commit implementing that.
There was a problem hiding this comment.
I think this proposed solution is good.
From our offline discussion aboout line 801 on whether it should be isXtn2 & (i < (I / 2)) or isXtn2 && (i < (I / 2)), given both arguments are bools the functionality would be the same, but using && may be marginally clearer of this fact?
| @@ -128,7 +128,7 @@ class RegisterValue { | |||
|
|
|||
| /** The underlying local member value. Aligned to 8 bytes to prevent | |||
| * potential alignment issue when casting. */ | |||
| alignas(8) char value[MAX_LOCAL_BYTES]; | |||
| alignas(8) char localValue[MAX_LOCAL_BYTES] = {}; | |||
There was a problem hiding this comment.
This is slightly inefficient as it initialises localValue to all 0's for every instance which will be overwritten when constructing the instance. We should test if there is any visible performance degradation from this change. Maybe another one for Leo @ABenC377
There was a problem hiding this comment.
Alternative solution is to do something like
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wuninitialized"
foo(b); /* no diagnostic for this one */
#pragma GCC diagnostic pop
There was a problem hiding this comment.
Given we use Pool's for RegisterValues this may not cause a huge performance hit. Worth checking though
| @@ -128,7 +128,7 @@ class RegisterValue { | |||
|
|
|||
| /** The underlying local member value. Aligned to 8 bytes to prevent | |||
| * potential alignment issue when casting. */ | |||
| alignas(8) char value[MAX_LOCAL_BYTES]; | |||
| alignas(8) char localValue[MAX_LOCAL_BYTES] = {}; | |||
There was a problem hiding this comment.
Given we use Pool's for RegisterValues this may not cause a huge performance hit. Worth checking though
| const N* n; | ||
| if (isXtn2) { | ||
| d = sourceValues[0].getAsVector<D>(); | ||
| n = sourceValues[1].getAsVector<N>(); | ||
| } else { | ||
| d = {}; | ||
| n = sourceValues[0].getAsVector<N>(); | ||
| } | ||
|
|
||
| D out[16 / sizeof(D)] = {0}; | ||
| int index = 0; | ||
|
|
||
| for (int i = 0; i < I; i++) { | ||
| if (isXtn2 & (i < (I / 2))) { | ||
| assert(isXtn2 && "isXtn2 is false so d is not initialised"); | ||
| out[i] = d[i]; | ||
| } else { | ||
| out[i] = static_cast<D>(n[index]); |
There was a problem hiding this comment.
I think this proposed solution is good.
From our offline discussion aboout line 801 on whether it should be isXtn2 & (i < (I / 2)) or isXtn2 && (i < (I / 2)), given both arguments are bools the functionality would be the same, but using && may be marginally clearer of this fact?
There was a problem hiding this comment.
All looks good pending the performance check.
Is it worth including I didn't spot the separate PR, ignore!cstdint into Register.hh to also fix that issue, or will we leave it for the SME branch? Imo we may as well add it now to get rid of another potential error point.
|
#rerun tests |
Fixes #426