[PATCH] MC: Support larger COFF string tables
Rafael Ávila de Espíndola
rafael.espindola at gmail.com
Mon Apr 15 07:25:55 PDT 2013
The implementation looks fine with just some nits.
Just wait for Bigcheese to comment on the format itself.
Thanks!
================
Comment at: lib/Object/COFFObjectFile.cpp:77
@@ +76,3 @@
+ // invalid.
+ if (CharVal >= 64)
+ return true;
----------------
You just computed CharVal. You can just assert that it is < 64, no?
================
Comment at: lib/Object/COFFObjectFile.cpp:85
@@ +84,3 @@
+ // Check for overflow by shifting back and seeing if bits were lost.
+ if (Result/64 < PrevResult)
+ return true;
----------------
Why not do the math with an uint64_t? With that you assert this too, no? Since you have an assert on the Str size being <= 6, you get at most 6 * log2(64) bits (36). That way you only need one check outside the loop if the result must fit in 32 bits.
http://llvm-reviews.chandlerc.com/D667
More information about the llvm-commits
mailing list