Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Jeff Law

On 07/15/14 23:31, Andi Kleen wrote:

From: Andi Kleen a...@linux.intel.com

No substantial changes, although the hash values will be slightly
different.

gcc/:

2014-07-10  Andi Kleen  a...@linux.intel.com

* lto-streamer-out.c (hash_tree): Convert to inchash.
(add_flag): New macro.
So my question here, does this make any existing LTO objects no longer 
usable?  If so, what, if any policy do we have when we make that kind of 
change?


If existing LTO objects continue to work, then this patch is fine too 
once the explicit begin/end vs ctor/dtor stuff is fixed.



jeff




Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Richard Biener
On Wed, Jul 23, 2014 at 5:40 AM, Jeff Law l...@redhat.com wrote:
 On 07/15/14 23:31, Andi Kleen wrote:

 From: Andi Kleen a...@linux.intel.com

 No substantial changes, although the hash values will be slightly
 different.

 gcc/:

 2014-07-10  Andi Kleen  a...@linux.intel.com

 * lto-streamer-out.c (hash_tree): Convert to inchash.
 (add_flag): New macro.

 So my question here, does this make any existing LTO objects no longer
 usable?  If so, what, if any policy do we have when we make that kind of
 change?

 If existing LTO objects continue to work, then this patch is fine too once
 the explicit begin/end vs ctor/dtor stuff is fixed.

I'd say add_flag should be in the hash abstraction?  You convert
only some to hash_flag and not all of them - why?  For example
I don't like your handling of TYPE/DECL_MODE.  Please refrain
from doing semantical changes like that and call it a simple
refactoring.

Richard.


 jeff




Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Andi Kleen
On Tue, Jul 22, 2014 at 09:40:15PM -0600, Jeff Law wrote:
 On 07/15/14 23:31, Andi Kleen wrote:
 From: Andi Kleen a...@linux.intel.com
 
 No substantial changes, although the hash values will be slightly
 different.
 
 gcc/:
 
 2014-07-10  Andi Kleen  a...@linux.intel.com
 
  * lto-streamer-out.c (hash_tree): Convert to inchash.
  (add_flag): New macro.
 So my question here, does this make any existing LTO objects no longer
 usable?  If so, what, if any policy do we have when we make that kind of
 change?

It breaks the format because a few type hashes change

But breaks happen all the time during development. Currently just adding
a new command option breaks the format. Or pretty much any change to
gimple.

Generally in my experience LTO object files don't stay usable more
than a few days in phase 1.

Currently the format is even broken for patch level releases, usually
due to some option change.

The policy is just that for a given release that is unchanged the 
object format stays compatible.

I submitted some patches to improve this slightly for the options, 
but they were not accepted because of all the other issues.
https://gcc.gnu.org/ml/gcc-patches/2010-10/msg00099.html

Given that reality I assume the patch is ok for you?

-Andi

a...@linux.intel.com -- Speaking for myself only


Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Jan Hubicka
 On 07/15/14 23:31, Andi Kleen wrote:
 From: Andi Kleen a...@linux.intel.com
 
 No substantial changes, although the hash values will be slightly
 different.
 
 gcc/:
 
 2014-07-10  Andi Kleen  a...@linux.intel.com
 
  * lto-streamer-out.c (hash_tree): Convert to inchash.
  (add_flag): New macro.
 So my question here, does this make any existing LTO objects no
 longer usable?  If so, what, if any policy do we have when we make
 that kind of change?

We are freely breaking LTO objects all the time and have version checks
that are bumped after each release. If you mix object files from released
versions you get correct diagnostics, if you mix object files from different
versions of trunk, you get random errors or ICEs.  I think we managed to
stay bytecode compatible for 4.8 release series. (Richi knows better)

Once we get well defined gimple and types, we can get also stable bytecode,
but until that this seems resonable policy.

Honza
 
 If existing LTO objects continue to work, then this patch is fine
 too once the explicit begin/end vs ctor/dtor stuff is fixed.
 
 
 jeff
 


Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Andi Kleen
 I think we managed to
 stay bytecode compatible for 4.8 release series. (Richi knows better)

Nope, fortran options broke it at some point.

-Andi
-- 
a...@linux.intel.com -- Speaking for myself only.


Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Richard Biener
On July 23, 2014 5:15:53 PM CEST, Andi Kleen a...@firstfloor.org wrote:
 I think we managed to
 stay bytecode compatible for 4.8 release series. (Richi knows better)

Nope, fortran options broke it at some point.

We try hard to not break things on branches or at least should bump the version 
if we do.  Maybe we need some mostly trivial bytecode test cases for branches...

Richard.

-Andi




Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Jan Hubicka
  I think we managed to
  stay bytecode compatible for 4.8 release series. (Richi knows better)
 
 Nope, fortran options broke it at some point.

Duh, option streaming sucks, another things that ought to be fixed this release 
cycle...

Honza


Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Andi Kleen
On Wed, Jul 23, 2014 at 06:00:35PM +0200, Richard Biener wrote:
 On July 23, 2014 5:15:53 PM CEST, Andi Kleen a...@firstfloor.org wrote:
  I think we managed to
  stay bytecode compatible for 4.8 release series. (Richi knows better)
 
 Nope, fortran options broke it at some point.
 
 We try hard to not break things on branches or at least should bump the 
 version if we do.  Maybe we need some mostly trivial bytecode test cases for 
 branches...

This patch would have caught that case:

https://gcc.gnu.org/ml/gcc-patches/2010-10/msg00099.html

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only.


Re: [PATCH 4/4] Convert lto streamer out hashing to inchash

2014-07-23 Thread Andi Kleen
On Wed, Jul 23, 2014 at 04:21:59PM +0200, Richard Biener wrote:
 On Wed, Jul 23, 2014 at 5:40 AM, Jeff Law l...@redhat.com wrote:
  On 07/15/14 23:31, Andi Kleen wrote:
 
  From: Andi Kleen a...@linux.intel.com
 
  No substantial changes, although the hash values will be slightly
  different.
 
  gcc/:
 
  2014-07-10  Andi Kleen  a...@linux.intel.com
 
  * lto-streamer-out.c (hash_tree): Convert to inchash.
  (add_flag): New macro.
 
  So my question here, does this make any existing LTO objects no longer
  usable?  If so, what, if any policy do we have when we make that kind of
  change?
 
  If existing LTO objects continue to work, then this patch is fine too once
  the explicit begin/end vs ctor/dtor stuff is fixed.
 
 I'd say add_flag should be in the hash abstraction?  You convert
 only some to hash_flag and not all of them - why? 

I'll do all.

 For example
 I don't like your handling of TYPE/DECL_MODE.  Please refrain
 from doing semantical changes like that and call it a simple
 refactoring.

Ok. I'll drop it.

Is it ok with those changes?
-Andi