1478671 - make *Value helpers constexpr-foldable...again

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Nathan Froyd [:froydnj]]

Bug 899309 and bug 899652, long ago, modified JS::Value initialization and friends so that various generated bindings would not cause static constructors on some platforms.  e.g. bug 899309 comment 1 shows simple numeric constant specifiers causing static constructors, which was fixed by making various things `constexpr`

Unfortunately, we seem to have changed the code in the interim, undoing the effects of those changes.  See for instance bug 1473465 comment 9, which shows simple numeric constant specifiers causing static constructors on MSVC.  (MSVC's constexpr handling is...wonky, so it's not clear that adding `constexpr` back will fix things.  But it will probably help on other platforms.)

Comment 1

[Nathan Froyd [:froydnj]]

Another alternative is to make the DOM bindings generate Value::fromInt32 and friends; it's not obvious to me what is supposed to be the public/preferred interface here...

Comment 2

[Jan de Mooij [:jandem]]

Note that JS::UndefinedValue() and JS::Int32Value(..) *are* constexpr still.

I'm not sure how hard it is to make JS::NumberValue constexpr. Another option is to make Codegen.py use Int32Value where it can (bug 1473465 comment 9 shows we're using NumberValue for {0,1,2,3}).

Comment 3

[Jan de Mooij [:jandem]]

Hm, NumberValue(uint32_t i) is constexpr already. Maybe MSVC is messing up again? We had similar issues with jsid in bug 1464036 :(

Comment 4

[Nathan Froyd [:froydnj]]

I can believe MSVC is botching things, yes. :(

Comment 5

[Nathan Froyd [:froydnj]]

Hm, looking at NumberValue(uint32_t), I see the conditional expression and ISTR that unless you take special steps, C++ specifies copies take place with such expressions?  Linux has a (small) static constructor for WebGL2RenderingContext.cpp for initializing what I assume is:

  { "INVALID_INDEX", JS::NumberValue(4294967295U) },

which is the only value > JSVAL_INT_MAX.

Comment 6

[Nathan Froyd [:froydnj]]

Alternative theory: constexpr-folding Value::fromDouble(double(i)) requires doing int->double conversion at compile time, and that doesn't get done properly in the C++ frontend, even though it might be done by the middle-end?

Comment 7

[Jason Orendorff [:jorendorff]]

Nathan, we're looking at this for triage and decided not to prioritize it. If the performance impact is a big deal, warranting urgent attention, please ni? me.

Comment 8

[Nathan Froyd [:froydnj]]

Not prioritizing this at the present time is fine!