Closed Kuree closed 5 years ago
Totals | |
---|---|
Change from base Build 928: | 0.01% |
Covered Lines: | 1368 |
Relevant Lines: | 1791 |
Interesting, is this because without the mask, C is promoting the byte (char) to a 32 bit type (used by verilator?) which results in sign extension? Then adding the mask results in the promotion to 32 bits without extension because the mask numeric literal is treated as a 32 bit value?
I believe so. char
in C/C++
is signed by default. So there might be a sign extension during the implicit conversation. Bitwise &
should correct this error regardless whether the input is signed or not.
Confirmed via testing!
// promotion.c
#include "printf.h"
int main() {
char byte = 0xEE;
printf("%%hhx byte = %hhx\n", byte);
printf("%%x byte = %x\n", byte);
printf("%%x (int) byte = %x\n", (int) byte);
printf("%%x byte & 0xFF = %x\n", byte & 0xFF);
}
and
/tmp
❯ gcc promotion.c && ./a.out
%hhx byte = ee
%x byte = ffffffee
%x (int) byte = ffffffee
%x byte & 0xFF = ee
Another thing we could try is using unsigned char
instead of char
for the input type of files. This design enforces that files are read as unsigned bytes. Are there cases where it makes sense to read file data as a signed byte ever?
I need to confirm this with @jeffsetter and other people who do the application development. For DNN the inputs may be signed depends on how they process it.
I was templating my testbench to support different types, including the signed and unsigned variants. Images support unsigned and signed types.
Whenever the high bit for the input is high, i.e. when the value is larger than
0x80
, the current fault will pad 1's to make an negative number. This patch use masks to mask them off.That's the cause that why
verilator
failed inGarnetFlow
butncsim
works: https://github.com/StanfordAHA/GarnetFlow/commits/masterEDIT: Travis version is working with this fix: https://travis-ci.com/StanfordAHA/GarnetFlow/builds/112535673