I'm certainly not expecting that type changes will magically make this code
64-bit clean.  If that were true, it would've been done already.

If it would be better, I can do these changes out of tree and come back with
things when I have them working.  My question from the previous email is
whether or not generic changes would be accepted into the tree or not - if
the answer is no, that's fine.

Jeff Bailey >|< Google, Inc. >|< +1 514 670-8754


On Thu, Jan 22, 2009 at 3:30 PM, Dean McNamee <[email protected]> wrote:

>
> When we've discussed the v8 pointer/type situation before, it seemed
> like there would need to be some discussion on typedefs and style so
> that the types could really make sense internally for V8.  It's hard,
> because there are pointers, pointers get tagged, they have certain
> bits used, and a lot of arithmetic can happen on them.  The GC code
> will only work for 32-bits.  All of the object tagging is designed
> only for 32-bits.
>
> Making V8 work on 64-bits is much much bigger than changing some types
> around and changing the scons.  It probably deserves some fairly big
> design discussions, and I would guess it would take at least a few
> months, and it will be hard not to fracture the code a bunch, with
> separate code for 32/64 bit (GC, object layout, etc).
>
> On Thu, Jan 22, 2009 at 4:21 PM, Jeff Bailey <[email protected]>
> wrote:
> >
> > Thanks!
> >
> > On x86_64 without -m32, this becomes a problem (as it would on ia64,
> > alpha, or other Debian architectures I might wind up porting this
> > to).  I've heard from others that 64 bit targets aren't really
> > interesting to the V8 team, so I assume I'll keep keeping some fixes
> > out of tree for now, but in cases where I can push patches in upstream
> > that won't get in the way, I'd like to.
> >
> > I have a patch in my tree to make doing this build easy while still
> > letting me test the 32bit build.  Debian and Ubuntu have a tool called
> > 'linux32' which will have the system report that it's running on ia32
> > rather than x86_64.  With the following patch, using "linux32 scons"
> > on amd64 produces the build the way you guys are used to and "scons"
> > attempts to build it for x86_64.  Note that I haven't tested this on
> > arm as I don't have that environment setup.  I don't remember off hand
> > if arm-linux-gcc accepts -m32.  If not, I would need to tweak this
> > further.  I also think the hack to tools/utils.py is probably wrong.
> > platform.machine() seems to always return the value we want, except
> > for i[3-7]86 where we want ia32.  I suspect it should just test for
> > that and otherwise return whatever was handed to it.  Distros and
> > whatnot would usually expect to use linux32 and a 32bit chroot to do
> > ia32 builds.
> >
> > Patch follows:
> >
> > Index: tools/utils.py
> > ===================================================================
> > --- tools/utils.py      (revision 1129)
> > +++ tools/utils.py      (working copy)
> > @@ -63,6 +63,8 @@
> >   id = platform.machine()
> >   if id.startswith('arm'):
> >     return 'arm'
> > +  if id.startswith('x86_64'):
> > +    return 'x86_64'
> >   elif (not id) or (not re.match('(x|i[3-6])86', id) is None):
> >     return 'ia32'
> >   else:
> > Index: SConstruct
> > ===================================================================
> > --- SConstruct  (revision 1129)
> > +++ SConstruct  (working copy)
> > @@ -57,10 +57,10 @@
> >     'os:freebsd': {
> >       'LIBS':         ['execinfo']
> >     },
> > -    'wordsize:64': {
> > +    'wordsize:32': {
> >       'CCFLAGS':      ['-m32'],
> >       'LINKFLAGS':    ['-m32']
> > -    }
> > +    },
> >   },
> >   'msvc': {
> >     'all': {
> > @@ -175,7 +175,7 @@
> >     'all': {
> >       'LIBPATH': [abspath('.')]
> >     },
> > -    'wordsize:64': {
> > +    'wordsize:32': {
> >       'CCFLAGS':      ['-m32'],
> >       'LINKFLAGS':    ['-m32']
> >     },
> > @@ -204,7 +204,7 @@
> >     'os:freebsd': {
> >       'LIBS':         ['execinfo']
> >     },
> > -    'wordsize:64': {
> > +    'wordsize:32': {
> >       'CCFLAGS':      ['-m32'],
> >       'LINKFLAGS':    ['-m32']
> >     },
> > @@ -298,7 +298,7 @@
> >     'help': 'the os to build for'
> >   },
> >   'arch': {
> > -    'values':['arm', 'ia32'],
> > +    'values':['arm', 'ia32', 'x86_64'],
> >     'default': ARCH_GUESS,
> >     'help': 'the architecture to build for'
> >   },
> >
> >
> > On Jan 22, 9:03 am, Mads Sig Ager <[email protected]> wrote:
> >> Thanks for the patch!   Using %p is the right thing here.  It has been
> >> committed in V8 bleeding_edge revision 1128.  :-)
> >>
> >> Do you have a build setup where this is a problem?  If you do, we
> >> would be interested in more details of the setup so we can test
> >> against it.
> >>
> >> Cheers,    -- Mads
> >>
> >> On Wed, Jan 21, 2009 at 7:10 PM, Jeff Bailey <[email protected]>
> wrote:
> >>
> >> > Hi!  There seem to be a number of places that assume that a pointer is
> >> > the same size as an int (or an ILP32 system in general).  Will fixes
> >> > like this be accepted?
> >>
> >> > 2009-01-20  Jeff Bailey <[email protected]>
> >>
> >> >        * src/checks.h: Use %p modifier for displaying pointers.
> >>
> >> > Index: src/checks.h
> >> > ===================================================================
> >> > --- src/checks.h        (revision 1112)
> >> > +++ src/checks.h        (working copy)
> >> > @@ -136,9 +136,9 @@
> >> >                                      void* value) {
> >> >   if (expected != value) {
> >> >     V8_Fatal(file, line,
> >> > -             "CHECK_EQ(%s, %s) failed\n#   Expected: %i\n#   Found:
> >> > %i",
> >> > +             "CHECK_EQ(%s, %s) failed\n#   Expected: %p\n#   Found:
> >> > %p",
> >> >              expected_source, value_source,
> >> > -             reinterpret_cast<int>(expected), reinterpret_cast<int>
> >> > (value));
> >> > +             expected, value);
> >> >   }
> >> >  }
> >>
> >> > @@ -150,8 +150,8 @@
> >> >                                         const char* value_source,
> >> >                                         void* value) {
> >> >   if (expected == value) {
> >> > -    V8_Fatal(file, line, "CHECK_NE(%s, %s) failed\n#   Value: %i",
> >> > -             expected_source, value_source, reinterpret_cast<int>
> >> > (value));
> >> > +    V8_Fatal(file, line, "CHECK_NE(%s, %s) failed\n#   Value: %p",
> >> > +             expected_source, value_source, value);
> >> >   }
> >> >  }
> > >
> >
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to