Ship It!

src/arch/x86/tlb.cc (Diff revision 5)

I see that altAddr and defAddr are defined by Gabe's bitfield package, which I assume makes them unsigned.  Would it make more sense to change logSize to unsigned than to cast those fields to signed?

Show all issues

1. Nilay Vaish June 3, 2012, 9:33 a.m. (June 3, 2012, 9:33 a.m.)

   IIRC, gcc's error output did not say anything related to signed vs unsigned values.
   It was about these two bitfields representing different parts of entire thing. But
   I'll leave it to Jayneel to confirm whether my recollection is correct.

2. Jayneel Gandhi June 4, 2012, 1:21 a.m. (June 4, 2012, 1:21 a.m.)

   Yes, Nilay. You are right on that. gcc was not complaining about signed vs unsigned. It was complaining that the typecast was missing since we are extracting parts of a structure.

3. Jayneel Gandhi June 4, 2012, 1:26 a.m. (June 4, 2012, 1:26 a.m.)

   This was the error that I got from gcc.
   error: operands to ?: have different types 'BitfieldBackend::RegularBitfieldTypes<long unsigned int>::Bitfield<15, 14>' and 'BitfieldBackend::RegularBitfieldTypes<long unsigned int>::Bitfield<13, 12>'
   
   @Steve:
   I don't mind them making all of them unsigned int since logSize should never be negative. Is that fine.

4. Steve Reinhardt June 4, 2012, 7:15 a.m. (June 4, 2012, 7:15 a.m.)

   Thanks for the details, Jayneel.  I misunderstood the issue before.  I hadn't seen that error message previously.  It would be nice if there was a way to avoid the casting, but it would have to involve changing the Bitfield type somehow, and I don't know of an easy fix.
   
   It probably is better to make everything unsigned.
   

This line looks >80 chars; can you wrap it appropriately?  Also, I'm fine with using "unsigned" as an abbreviation for "unsigned int" (though that by itself isn't enough to get you below 80 chars). So something like this:

            unsigned logSize = sizeOverride ? (unsigned)m5Reg.altAddr
                                            : (unsigned)m5Reg.defAddr;

Ship It!