Use big-boy Types to annotate interface descriptor parameters

Use big-boy Types to annotate interface descriptor parameters

- Thread Type::FunctionType through stubs and the TF pipeline.
- Augment Typer to decorate parameter nodes with types from
  a Type::FunctionType associated with interface descriptors.
- Factor interface descriptors into platform-specific and
  platform-independent components so that all descriptors share
  a common Type::FunctionType for all platforms.

Committed: https://crrev.com/c019d7f498ce6fbac6659924e20ddb6c59aebeb8
Cr-Commit-Position: refs/heads/master@{#29248}

[danno, 2015-06-19 13:41:59]

Patchset #2 (id:20001) has been deleted

[danno, 2015-06-19 13:44:47]

Patchset #3 (id:60001) has been deleted

[danno, 2015-06-19 13:48:32]

danno@chromium.org changed reviewers:
+ mstarzinger@chromium.org, rossberg@chromium.org

[danno, 2015-06-19 14:07:44]

Please take a look
rossberg: Typer changes
michi: everything else

[rossberg, 2015-06-19 14:10:04]

rossberg@chromium.org changed reviewers:
+ bmeurer@chromium.org

[rossberg, 2015-06-19 14:20:49]

Typing parts look okay, but I wonder if TypeSignature isn't essentially
duplicating the existing FunctionType from types.h.

Adding Benedikt, who might also want to chime in.

https://codereview.chromium.org/1197703002/diff/100001/src/compiler/typer.h
File src/compiler/typer.h (right):

https://codereview.chromium.org/1197703002/diff/100001/src/compiler/typer.h#n...
src/compiler/typer.h:8: #include "src/compiler.h"
Unneeded include?

https://codereview.chromium.org/1197703002/diff/100001/src/type-signature.cc
File src/type-signature.cc (right):

https://codereview.chromium.org/1197703002/diff/100001/src/type-signature.cc#...
src/type-signature.cc:15: if (type->Is(Type::UntaggedSigned()) ||
type->Is(Type::UntaggedUnsigned())) {
Any reason you can't compare to UntaggedIntegral here?

[danno, 2015-06-19 15:19:00]

Let me rework this to use FunctionType, that does seem to make more sense.

[danno, 2015-06-20 00:24:56]

Patchset #5 (id:120001) has been deleted

[danno, 2015-06-20 00:25:11]

Patchset #4 (id:100001) has been deleted

[danno, 2015-06-20 00:34:54]

Feedback addressed. Please take another look.

[Benedikt Meurer, 2015-06-22 05:20:00]

bmeurer@chromium.org changed reviewers:
+ jarin@chromium.org

[Benedikt Meurer, 2015-06-22 05:27:28]

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc#newc...
src/code-stubs.cc:1050: Representation RepresentationFromType(Type* type) {
So UntaggedFloat64 and UntaggedFloat32 are mapped to Representation::Tagged?
That should be fixed to Representation::Double even if we don't use that
currently.

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h#newcode292
src/compiler.h:292: void SetFunctionType(Type::FunctionType* function_type) {
Nit: I think Type::FunctionType is an implementation detail of the type system
and should not be exposed? Use Type* instead.

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h#newcode488
src/compiler.h:488: Type::FunctionType* function_type_;
See above.

[Jarin, 2015-06-22 06:51:29]

Just nits from side. Otherwise, lgtm.

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc#newc...
src/code-stubs.cc:1050: Representation RepresentationFromType(Type* type) {
On 2015/06/22 05:27:28, Benedikt Meurer wrote:
> So UntaggedFloat64 and UntaggedFloat32 are mapped to Representation::Tagged?
> That should be fixed to Representation::Double even if we don't use that
> currently.

I would slightly prefer if handled only the cases that we really have and then
do UNREACHABLE for the cases we should not get (such as UntaggedFloat64).

https://codereview.chromium.org/1197703002/diff/140001/src/interface-descript...
File src/interface-descriptors.cc (right):

https://codereview.chromium.org/1197703002/diff/140001/src/interface-descript...
src/interface-descriptors.cc:14: static Type* SmiType() {
I believe you do not have to say 'static' if you are already in an anonymous
namespace.

[danno, 2015-06-23 14:39:45]

Feedback addressed, landing

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc
File src/code-stubs.cc (right):

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc#newc...
src/code-stubs.cc:1050: Representation RepresentationFromType(Type* type) {
On 2015/06/22 06:51:29, jarin wrote:
> On 2015/06/22 05:27:28, Benedikt Meurer wrote:
> > So UntaggedFloat64 and UntaggedFloat32 are mapped to Representation::Tagged?
> > That should be fixed to Representation::Double even if we don't use that
> > currently.
> 
> I would slightly prefer if handled only the cases that we really have and then
> do UNREACHABLE for the cases we should not get (such as UntaggedFloat64).

Done.

https://codereview.chromium.org/1197703002/diff/140001/src/code-stubs.cc#newc...
src/code-stubs.cc:1050: Representation RepresentationFromType(Type* type) {
On 2015/06/22 05:27:28, Benedikt Meurer wrote:
> So UntaggedFloat64 and UntaggedFloat32 are mapped to Representation::Tagged?
> That should be fixed to Representation::Double even if we don't use that
> currently.

I think I prefer Jaro's suggestion for now, since we don't test these cases and
I'd rather not build in code that isn't actually used.

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h#newcode292
src/compiler.h:292: void SetFunctionType(Type::FunctionType* function_type) {
On 2015/06/22 05:27:28, Benedikt Meurer wrote:
> Nit: I think Type::FunctionType is an implementation detail of the type system
> and should not be exposed? Use Type* instead.

As discussed over chat with you and Jaro, I'm going to leave this as-is for now.

https://codereview.chromium.org/1197703002/diff/140001/src/interface-descript...
File src/interface-descriptors.cc (right):

https://codereview.chromium.org/1197703002/diff/140001/src/interface-descript...
src/interface-descriptors.cc:14: static Type* SmiType() {
On 2015/06/22 06:51:29, jarin wrote:
> I believe you do not have to say 'static' if you are already in an anonymous
> namespace.

Done.

[danno, 2015-06-24 06:19:33]

The CQ bit was checked by danno@chromium.org

[danno, 2015-06-24 06:19:34]

The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1197703002/#ps180001
(title: "Latest")

[commit-bot: I haz the power, 2015-06-24 06:19:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1197703002/180001

[commit-bot: I haz the power, 2015-06-24 06:21:52]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2015-06-24 06:22:15]

Patchset 6 (id:??) landed as
https://crrev.com/c019d7f498ce6fbac6659924e20ddb6c59aebeb8
Cr-Commit-Position: refs/heads/master@{#29248}

[rossberg, 2015-06-29 12:29:34]

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h
File src/compiler.h (right):

https://codereview.chromium.org/1197703002/diff/140001/src/compiler.h#newcode292
src/compiler.h:292: void SetFunctionType(Type::FunctionType* function_type) {
On 2015/06/23 14:39:44, danno wrote:
> On 2015/06/22 05:27:28, Benedikt Meurer wrote:
> > Nit: I think Type::FunctionType is an implementation detail of the type
system
> > and should not be exposed? Use Type* instead.
> 
> As discussed over chat with you and Jaro, I'm going to leave this as-is for
now.

FunctionType is not an implementation detail -- it is the public interface class
to get to a function type's attributes. Similarly for the other structural
types. The only internal classes are BitsetType, StructuralType, and UnionType
(see the respective comments at the beginning of TypeImpl).