Tue, Dec 06, 2022 at 01:24:09PM +0200, Marko Mäkelä wrote:
The first attached patch includes your suggested fixes and nothing that you opposed so far. The second attached patch fixes the following 2 issues. I agree that the NumCamSlots==0 case could be solved in a nicer way.
I tried to make sense out of the generated code, and I did not find any evidence of a subsequent unsafe optimization. Still, I think that it is a good practice to address any compiler warnings even when they are genuinely bogus, because often such warnings can identify real trouble. The only question should be whether silencing the warnings could introduce an unacceptable amount of runtime overhead.
Maybe the simplest way to silence the warning would be to bloat the variable-length array with 1 extra element, wasting sizeof(int) bytes of stack space:
int SlotPriority[NumCamSlots + 1];
That would avoid adding a conditional branch, like the one that would have been part of my initially suggested std::max(NumCamSlots, 1). It might not even translate into an extra machine instruction, if the constant can be embedded in an instruction that would be generated anyway.
I tested the following alternative patch. It is duplicating a lot of source code, which I usually find a bad idea, including in this case. Some "imp <<= 1" or "imp <<= 8" are unnecessary and could be omitted. I retained the statements to keep the transformed code more similar to what it was copied and adapted from.
Marko