| Message ID | 1413369519-11677-5-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
On Wed, Oct 15, 2014 at 04:38:33AM -0600, Simon Glass wrote: > Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily > compatible on 64-bit machines. Use the correct typedef instead of > writing the supposed type out in full. > > Signed-off-by: Simon Glass <sjg@chromium.org> Applied to u-boot/master, thanks!
Hi Simon, 2014-10-15 19:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily > compatible on 64-bit machines. Use the correct typedef instead of > writing the supposed type out in full. I doubt this statement. I think "unsigned long long" always has 64bit width. (C specification guarantees that the width of "unsigned long long" is greater or equal to 64 bit) Could you tell me which toolchain violates it?
Hi Masahiro, On 15 December 2014 at 09:55, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: > Hi Simon, > > > 2014-10-15 19:38 GMT+09:00 Simon Glass <sjg@chromium.org>: >> Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily >> compatible on 64-bit machines. Use the correct typedef instead of >> writing the supposed type out in full. > > I doubt this statement. > > I think "unsigned long long" always has 64bit width. > > (C specification guarantees that the width of "unsigned long long" > is greater or equal to 64 bit) > > Could you tell me which toolchain violates it? Some compilers use 'unsigned long' and some use 'unsigned long long'. I think that is the core of the problem. We don't have a problem with unsigned long long not being at least 64-bit. I wonder whether some toolchains use 128-bit for this? Regards, Simon
Hi Simon, 2014-12-16 3:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > Hi Masahiro, > > On 15 December 2014 at 09:55, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: >> Hi Simon, >> >> >> 2014-10-15 19:38 GMT+09:00 Simon Glass <sjg@chromium.org>: >>> Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily >>> compatible on 64-bit machines. Use the correct typedef instead of >>> writing the supposed type out in full. >> >> I doubt this statement. >> >> I think "unsigned long long" always has 64bit width. >> >> (C specification guarantees that the width of "unsigned long long" >> is greater or equal to 64 bit) >> >> Could you tell me which toolchain violates it? > > Some compilers use 'unsigned long' and some use 'unsigned long long'. > I think that is the core of the problem. > > We don't have a problem with unsigned long long not being at least > 64-bit. I wonder whether some toolchains use 128-bit for this? That is not my point. "unsigned long long" has 64-bit width whether on 32bit compilers or 64bit compilers or whatever. We should always hard-code the definition: typedef unsigned long long uint64_t; That's all. We can always use "%llx" for printing uint64_t or u64. (and this is what U-boot (and Linux) had used until you broke the consistency.) If we include <stdint.h>, you are right. It is the beginning of nightmare. Some compilers use "unsigned long" for uint64_t and some use "unsigned long long" for uint64_t. What did it buy us? You just introduced unreadability by using PRIu64 for printing 64bit width variables. Best Regards Masahiro Yamada
Hi Masahiro, On 15 December 2014 at 18:38, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: > > Hi Simon, > > > 2014-12-16 3:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > > Hi Masahiro, > > > > On 15 December 2014 at 09:55, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: > >> Hi Simon, > >> > >> > >> 2014-10-15 19:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > >>> Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily > >>> compatible on 64-bit machines. Use the correct typedef instead of > >>> writing the supposed type out in full. > >> > >> I doubt this statement. > >> > >> I think "unsigned long long" always has 64bit width. > >> > >> (C specification guarantees that the width of "unsigned long long" > >> is greater or equal to 64 bit) > >> > >> Could you tell me which toolchain violates it? > > > > Some compilers use 'unsigned long' and some use 'unsigned long long'. > > I think that is the core of the problem. > > > > We don't have a problem with unsigned long long not being at least > > 64-bit. I wonder whether some toolchains use 128-bit for this? > > That is not my point. > > "unsigned long long" has 64-bit width whether on 32bit compilers > or 64bit compilers or whatever. I think that might be true at least for gcc. But in principle a 64-bit machine should use 128-bit for long long. > > > We should always hard-code the definition: > typedef unsigned long long uint64_t; > > That's all. We can always use "%llx" for printing uint64_t or u64. > (and this is what U-boot (and Linux) had used until you broke the consistency.) > > > If we include <stdint.h>, you are right. It is the beginning of nightmare. > Some compilers use "unsigned long" for uint64_t and some use > "unsigned long long" > for uint64_t. > > What did it buy us? > > You just introduced unreadability by using PRIu64 for printing 64bit > width variables. I have also hit this problem with m68k and one other compiler in U-Boot. Is it because these compilers are broken, or something else? Regards, Simon
Hi Simon, On Tue, 16 Dec 2014 21:38:34 -0700 Simon Glass <sjg@chromium.org> wrote: > Hi Masahiro, > > On 15 December 2014 at 18:38, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: > > > > Hi Simon, > > > > > > 2014-12-16 3:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > > > Hi Masahiro, > > > > > > On 15 December 2014 at 09:55, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote: > > >> Hi Simon, > > >> > > >> > > >> 2014-10-15 19:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > > >>> Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily > > >>> compatible on 64-bit machines. Use the correct typedef instead of > > >>> writing the supposed type out in full. > > >> > > >> I doubt this statement. > > >> > > >> I think "unsigned long long" always has 64bit width. > > >> > > >> (C specification guarantees that the width of "unsigned long long" > > >> is greater or equal to 64 bit) > > >> > > >> Could you tell me which toolchain violates it? > > > > > > Some compilers use 'unsigned long' and some use 'unsigned long long'. > > > I think that is the core of the problem. > > > > > > We don't have a problem with unsigned long long not being at least > > > 64-bit. I wonder whether some toolchains use 128-bit for this? > > > > That is not my point. > > > > "unsigned long long" has 64-bit width whether on 32bit compilers > > or 64bit compilers or whatever. > > I think that might be true at least for gcc. But in principle a 64-bit > machine should use 128-bit for long long. 128-bit variable? Are you kidding? I am not talking about "in principle" things, but talking about real compilers. So, on which compiler do you have problems? For instance, please? > > > > > > We should always hard-code the definition: > > typedef unsigned long long uint64_t; > > > > That's all. We can always use "%llx" for printing uint64_t or u64. > > (and this is what U-boot (and Linux) had used until you broke the consistency.) > > > > > > If we include <stdint.h>, you are right. It is the beginning of nightmare. > > Some compilers use "unsigned long" for uint64_t and some use > > "unsigned long long" > > for uint64_t. > > > > What did it buy us? > > > > You just introduced unreadability by using PRIu64 for printing 64bit > > width variables. > > I have also hit this problem with m68k and one other compiler in > U-Boot. Is it because these compilers are broken, or something else? I guess you are mentioning "size_t" problem on m68k. http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/188121/focus=188932 If so, you are already confused. "size_t" is another problem that should be discussed separetely. Notice [1] uint32_t, int32_t, uint64_t, int64_t, uintptr_t are provided by <stdint.h> [2] PRIx32, PRIx64, PRId32, PRId64 etc. are provided by <inttypes.h> [3] size_t is provided by <stddef.h> We are talking about [1] and [2]. And also notice [1] and [2] should be provided by the same compiler to work correctly. [3] should not be mixed up with [1]. If you are interested in the topic about the conflict between "size_t" type and "%z", I can introduce you another thread. But I am not showing that, in case this discussion goes wrong. Best Regards Masahiro Yamada
diff --git a/include/common.h b/include/common.h index d8584dd..02befa6 100644 --- a/include/common.h +++ b/include/common.h @@ -776,7 +776,7 @@ void invalidate_dcache_all(void); void invalidate_icache_all(void); /* arch/$(ARCH)/lib/ticks.S */ -unsigned long long get_ticks(void); +uint64_t get_ticks(void); void wait_ticks (unsigned long); /* arch/$(ARCH)/lib/time.c */ diff --git a/lib/time.c b/lib/time.c index c7b0264..fe43ccb 100644 --- a/lib/time.c +++ b/lib/time.c @@ -41,7 +41,7 @@ unsigned long notrace timer_read_counter(void) extern unsigned long __weak timer_read_counter(void); #endif -unsigned long long __weak notrace get_ticks(void) +uint64_t __weak notrace get_ticks(void) { unsigned long now = timer_read_counter(); @@ -49,11 +49,11 @@ unsigned long long __weak notrace get_ticks(void) if (now < gd->timebase_l) gd->timebase_h++; gd->timebase_l = now; - return ((unsigned long long)gd->timebase_h << 32) | gd->timebase_l; + return ((uint64_t)gd->timebase_h << 32) | gd->timebase_l; } /* Returns time in milliseconds */ -static unsigned long long notrace tick_to_time(unsigned long long tick) +static uint64_t notrace tick_to_time(uint64_t tick) { ulong div = get_tbclk(); @@ -78,9 +78,9 @@ unsigned long __weak notrace timer_get_us(void) return tick_to_time(get_ticks() * 1000); } -static unsigned long long usec_to_tick(unsigned long usec) +static uint64_t usec_to_tick(unsigned long usec) { - unsigned long long tick = usec; + uint64_t tick = usec; tick *= get_tbclk(); do_div(tick, 1000000); return tick; @@ -88,7 +88,7 @@ static unsigned long long usec_to_tick(unsigned long usec) void __weak __udelay(unsigned long usec) { - unsigned long long tmp; + uint64_t tmp; tmp = get_ticks() + usec_to_tick(usec); /* get current timestamp */
Unfortunately 'unsigned long long' and 'uint64_t' are not necessarily compatible on 64-bit machines. Use the correct typedef instead of writing the supposed type out in full. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/common.h | 2 +- lib/time.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-)