[zscript] GCC/Clang compiling errors

[Edward-san]

Code: Select all

[ 84%] Building CXX object src/CMakeFiles/zdoom.dir/scripting/codegeneration/codegen.cpp.o
/home/edward-san/zdoom/branch/zscript/src/scripting/codegeneration/codegen.cpp: In member function ‘virtual ExpEmit FxArrayElement::Emit(VMFunctionBuilder*)’:
/home/edward-san/zdoom/branch/zscript/src/scripting/codegeneration/codegen.cpp:6626:21: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    if (start.RegNum <= 255)
        ~~~~~~~~~~~~~^~~~~~
[ 32%] Building CXX object src/CMakeFiles/zdoom.dir/scripting/vm/vmexec.cpp.o
In file included from /home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.cpp:117:0:
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h: In static member function ‘static int VMExec_Checked::Exec(VMFrameStack*, const VMOP*, VMReturn*, int)’:
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h:233:4: error: label ‘MULVF3_RK’ used but not defined
 xx(MULVF3_RK, mulv3, RVRVKF,  MULVF3_RR,4, REGT_FLOAT),
    ^
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.h:11:42: note: in definition of macro ‘xx’
 #define xx(op,sym,mode,alt,kreg,ktype) &&op
                                          ^~
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h:235:4: error: label ‘DIVVF3_RK’ used but not defined
 xx(DIVVF3_RK, divv3, RVRVKF,  DIVVF3_RR,4, REGT_FLOAT),
    ^
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.h:11:42: note: in definition of macro ‘xx’
 #define xx(op,sym,mode,alt,kreg,ktype) &&op
                                          ^~
In file included from /home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.cpp:130:0:
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h: In static member function ‘static int VMExec_Unchecked::Exec(VMFrameStack*, const VMOP*, VMReturn*, int)’:
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h:233:4: error: label ‘MULVF3_RK’ used but not defined
 xx(MULVF3_RK, mulv3, RVRVKF,  MULVF3_RR,4, REGT_FLOAT),
    ^
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.h:11:42: note: in definition of macro ‘xx’
 #define xx(op,sym,mode,alt,kreg,ktype) &&op
                                          ^~
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmops.h:235:4: error: label ‘DIVVF3_RK’ used but not defined
 xx(DIVVF3_RK, divv3, RVRVKF,  DIVVF3_RR,4, REGT_FLOAT),
    ^
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmexec.h:11:42: note: in definition of macro ‘xx’
 #define xx(op,sym,mode,alt,kreg,ktype) &&op
                                          ^~

[Edward-san]

It seems I forgot to report Clang's error:

Code: Select all

[ 33%] Building CXX object src/CMakeFiles/zdoom.dir/scripting/vm/vmbuilder.cpp.o
/home/edward-san/zdoom/branch/zscript/src/scripting/vm/vmbuilder.cpp:888:85: error: 
      cannot pass object of non-trivial type 'FString' through variadic method;
      call will abort at runtime [-Wnon-pod-varargs]
  ..."%s in %s", err.GetMessage(), item.PrintableName);
                                        ^

And also this part was not fixed yet:

Code: Select all

[ 84%] Building CXX object src/CMakeFiles/zdoom.dir/scripting/codegeneration/codegen.cpp.o
/home/edward-san/zdoom/branch/zscript/src/scripting/codegeneration/codegen.cpp: In member function ‘virtual ExpEmit FxArrayElement::Emit(VMFunctionBuilder*)’:
/home/edward-san/zdoom/branch/zscript/src/scripting/codegeneration/codegen.cpp:6626:21: warning: comparison is always true due to limited range of data type [-Wtype-limits]
    if (start.RegNum <= 255)
        ~~~~~~~~~~~~~^~~~~~

[Graf Zahl]

Argh!
This is again one of the things that has always baffled me: What's the point of squeezing every shitty value into the smallest possible variable, without any provision whatsoever that things may go wrong and an overflow happens? I already spent several hours yesterday on lifting the 256 constants limit and still encounter places where this stuff is stored in a byte to save a pitiful amount of memory in a temporary struct... The entire VM stuff is full of this crap. I can understand it to a certain degree where it is designed to limit the footprint during execution, but this is a short lived compile-time only data structure.

[Rachael]

Personally I try for smaller number structures for the sake of speed and saving memory, but I almost never go lower than 16-bit unless I am absolutely sure the operation I am going for will fit inside a byte and will never be extended beyond that.

There are times when I'd rather use float instead of double, but when doing ZDoom I've learned that tends to go horribly wrong when it's in the game sim. (It's safe to use floats in some parts of the renderer, though...)

It's not so much an obsession - it's just more "use the right tool for the job" - which obviously goes the opposite direction from what you've pointed out. Not everything has to be done with long/uint32_t and double. And sometimes, using bigger structs than you need can slow things down, especially with operations that can be repeated a million times a second.

[Graf Zahl]

I think that 'bigger is slower' is mostly a myth. Remember, when ZDoom got floatified, AActor nearly had a 50% size increase. The performance impact was non-existent. Saving space in structs may be useful if you are crunching large arrays, but not if each object is separately allocated, then you'll get your cache misses no matter what.
And accessing 16 bit values on x86 also has a speed impact.

But one rule I deem important is, to never use data types that can just hold the needed values and have no provisions for overflow cases, ESPECIALLY when the needed checks aren't properly done, which also was the case here. The VM just blanketly assumed that no function would ever need more than 256 constants of a certain type, and the only means to deal with this was a single assert. It also never considered the possibility that some badly behaved code can exceed the register store and just ran off with it without any error. And these are not programming errors that can be guarded with an assert, these are legitimate failure cases that need proper error handling. And then storing the value in a variable that has no room for error - sorry that's not ok.

[Rachael]

Yes - agreed, there. Even with still being less experienced with programming, it sometimes baffles me when I see smaller data types, knowing full well that a value can be inputted to cause an overflow.

[kevans91]

Hi,

Pull request for the Clang error:
cannot pass object of non-trivial type 'FString' through variadic method;

[Major Cooke]

Might be better off in a new thread.

[kevans91]

Might be better off in a new thread.

It was mentioned in one of Edward's initial posts on this one, so it seemed fitting.