[PATCH] D84054: [lld][ELF] Add LOG2CEIL builtin ldscript function

Sat Jul 25 01:20:35 PDT 2020

grimar added a comment.

In D84054#2172962 <https://reviews.llvm.org/D84054#2172962>, @irichter wrote:

> In D84054#2171513 <https://reviews.llvm.org/D84054#2171513>, @grimar wrote:
>
> > I do not think we should focus too much on this in LLD test. I think just a few values should be fine. E.g 0, 1, 2, 3, 4, 65535, 65536, uint64(-1)
> >  @maskray, what do you think?
>
>
> I see that you suggest testing the upper bounds for 16-bits (`UINT16_MAX` and `UINT16_MAX+1`, both which should return 16). Is there any particular reason for checking this at this boundary? Why not `UINT32_MAX`,`UINT32_MAX+1` (either instead of UINT16_MAX, or in addition thereto)? Should I also be testing `UINT16_MAX+2` (should return 17) and/or `UINT32_MAX+2` (should return 33)?

I was thinking about just showing that we produce an expected result for some set of arbitrary values.
My range contains 0 (min possible value) + a few values at start to demonstrate the behavior, 2 arbitrary values somewhere in the middle just in case to confirm the behavior and the max possible value.

Regarding `UINT16_MAX` and `UINT16_MAX+1`, it was my mistake, sorry, `UINT16_MAX+1` and `UINT16_MAX+2`, should be better to show the different result.
I also do not think it is important to stick to UINT16_MAX instead of UINT32_MAX, the idea was to test values that are not at the start nor the end of a range. So any values can be used I think.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84054/new/

https://reviews.llvm.org/D84054