public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] Implement new hook for max_align_t_align
@ 2016-10-08 16:43 Bernd Edlinger
  0 siblings, 0 replies; 34+ messages in thread
From: Bernd Edlinger @ 2016-10-08 16:43 UTC (permalink / raw)
  To: John David Anglin; +Cc: gcc-patches

Hi,

I think your callback should also directly control the
alignment of max_align_t in stddef.h:

typedef struct {
   long long __max_align_ll __attribute__((__aligned__(__alignof__(long 
long))));
   long double __max_align_ld 
__attribute__((__aligned__(__alignof__(long double))));
   /* _Float128 is defined as a basic type, so max_align_t must be
      sufficiently aligned for it.  This code must work in C++, so we
      use __float128 here; that is only available on some
      architectures, but only on i386 is extra alignment needed for
      __float128.  */
#ifdef __i386__
   __float128 __max_align_f128 
__attribute__((__aligned__(__alignof(__float128))));
#endif
} max_align_t;


otherwise these will not match.


Bernd.

^ permalink raw reply	[flat|nested] 34+ messages in thread
* Re: [PATCH] Implement new hook for max_align_t_align
@ 2016-10-08 17:01 Bernd Edlinger
  2016-10-08 17:36 ` John David Anglin
  0 siblings, 1 reply; 34+ messages in thread
From: Bernd Edlinger @ 2016-10-08 17:01 UTC (permalink / raw)
  To: John David Anglin; +Cc: gcc-patches, Jeff Law, Jason Merrill

bounced, resent...

Hi,

I think your callback should also directly control the
alignment of max_align_t in stddef.h:

typedef struct {
   long long __max_align_ll __attribute__((__aligned__(__alignof__(long 
long))));
   long double __max_align_ld 
__attribute__((__aligned__(__alignof__(long double))));
   /* _Float128 is defined as a basic type, so max_align_t must be
      sufficiently aligned for it.  This code must work in C++, so we
      use __float128 here; that is only available on some
      architectures, but only on i386 is extra alignment needed for
      __float128.  */
#ifdef __i386__
   __float128 __max_align_f128 
__attribute__((__aligned__(__alignof(__float128))));
#endif
} max_align_t;


otherwise these will not match.

I think is __alignof(max_aling_t) == max_align_t_align ()
on your target?


Bernd.

^ permalink raw reply	[flat|nested] 34+ messages in thread
* [PATCH] Implement new hook for max_align_t_align
@ 2016-10-08 15:45 John David Anglin
  0 siblings, 0 replies; 34+ messages in thread
From: John David Anglin @ 2016-10-08 15:45 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jeff Law, jason

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Various pthread types on hppa-linux require an alignment of 16 bytes for ABI reasons.  This is larger
than the current implementations of max_align_t and max_align_t_align.  As a result, the patch to
implement C++17 over-aligned new  causes warnings on hppa.  The thread here discusses the problem:
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg01593.html

The attached patch implements a new hook to allow overriding the default implementation of max_align_t_align
in the PA backend.  It changes the max_align_t alignment to 16 bytes on 32-bit hppa-linux.  I added an additional
comment to BIGGEST_ALIGNMENT in pa.h to document the alignment choices for long double on hpux.  I also
defined MALLOC_ABI_ALIGNMENT so that it reflects the behaviour of the glibc and hpux malloc implementations.

Tested on hppa2.0w-hp-hpux11.11, hppa64-hp-hpux11.11 and hppa-unknown-linux-gnu with no observed
regressions.

Okay for trunk?

Dave
--
John David Anglin	dave.anglin@bell.net


[-- Attachment #2: ldcw-align.d.txt --]
[-- Type: text/plain, Size: 10431 bytes --]

2016-10-08  John David Anglin  <danglin@gcc.gnu.org>

gcc/c-family/
	* c-common.c (max_align_t_align): Move to targhooks.c.
	* c-common.h (max_align_t_align): Delete.
gcc/
	* target.def (max_align_t_align): New target hook.
	* targhooks.c (default_max_align_t_align): New.
	* targhooks.h (default_max_align_t_align): Declare.
	* config/pa/pa.h (BIGGEST_ALIGNMENT): Adjust comment.
	(MALLOC_ABI_ALIGNMENT): Define.
	* config/pa/pa.c (pa_max_align_t_align): New.
	(TARGET_MAX_ALIGN_T_ALIGN): Define.
	* doc/tm.texi.in (TARGET_MAX_ALIGN_T_ALIGN): Add documentation hook.
	* doc/tm.texi: Update.
gcc/cp/
	* decl.c (cxx_init_decl_processing): Use max_align_t_align target hook.
	* init.c (build_new_1): Likewise.

Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 240688)
+++ c-family/c-common.c	(working copy)
@@ -12899,22 +12899,6 @@
   return stv_nothing;
 }
 
-/* Return the alignment of std::max_align_t.
-
-   [support.types.layout] The type max_align_t is a POD type whose alignment
-   requirement is at least as great as that of every scalar type, and whose
-   alignment requirement is supported in every context.  */
-
-unsigned
-max_align_t_align ()
-{
-  unsigned int max_align = MAX (TYPE_ALIGN (long_long_integer_type_node),
-				TYPE_ALIGN (long_double_type_node));
-  if (float128_type_node != NULL_TREE)
-    max_align = MAX (max_align, TYPE_ALIGN (float128_type_node));
-  return max_align;
-}
-
 /* Return true iff ALIGN is an integral constant that is a fundamental
    alignment, as defined by [basic.align] in the c++-11
    specifications.
@@ -12928,7 +12912,7 @@
 bool
 cxx_fundamental_alignment_p (unsigned align)
 {
-  return (align <= max_align_t_align ());
+  return (align <= targetm.max_align_t_align ());
 }
 
 /* Return true if T is a pointer to a zero-sized aggregate.  */
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h	(revision 240688)
+++ c-family/c-common.h	(working copy)
@@ -864,7 +864,6 @@
 extern bool keyword_is_storage_class_specifier (enum rid);
 extern bool keyword_is_type_qualifier (enum rid);
 extern bool keyword_is_decl_specifier (enum rid);
-extern unsigned max_align_t_align (void);
 extern bool cxx_fundamental_alignment_p (unsigned);
 extern bool pointer_to_zero_sized_aggr_p (tree);
 extern bool diagnose_mismatched_attributes (tree, tree);
Index: config/pa/pa.c
===================================================================
--- config/pa/pa.c	(revision 240688)
+++ config/pa/pa.c	(working copy)
@@ -194,6 +194,7 @@
 static bool pa_legitimate_constant_p (machine_mode, rtx);
 static unsigned int pa_section_type_flags (tree, const char *, int);
 static bool pa_legitimate_address_p (machine_mode, rtx, bool);
+static unsigned int pa_max_align_t_align (void);
 
 /* The following extra sections are only used for SOM.  */
 static GTY(()) section *som_readonly_data_section;
@@ -400,6 +401,9 @@
 #undef TARGET_LRA_P
 #define TARGET_LRA_P hook_bool_void_false
 
+#undef TARGET_MAX_ALIGN_T_ALIGN
+#define TARGET_MAX_ALIGN_T_ALIGN pa_max_align_t_align
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 \f
 /* Parse the -mfixed-range= option string.  */
@@ -10719,4 +10723,16 @@
   return NULL_RTX;
 }
 
+/* The maximimum alignment in bits for the POD type std:max_align_t.
+   This is set to 128 on 32-bit non HP-UX systems to suppress warnings
+   about new with extended alignment.  This arises because various POSIX
+   types such as pthread_mutex_t have for historical reasons 128-bit
+   alignment but the default alignment of std:max_align_t is 64 bits.  */
+
+static unsigned int
+pa_max_align_t_align (void)
+{
+  return TARGET_HPUX && !TARGET_64BIT ? 64 : 128;
+}
+
 #include "gt-pa.h"
Index: config/pa/pa.h
===================================================================
--- config/pa/pa.h	(revision 240688)
+++ config/pa/pa.h	(working copy)
@@ -292,9 +292,21 @@
 /* A bit-field declared as `int' forces `int' alignment for the struct.  */
 #define PCC_BITFIELD_TYPE_MATTERS 1
 
-/* No data type wants to be aligned rounder than this.  */
+/* No data type wants to be aligned rounder than this.  The long double
+   type has 16-byte alignment on the 64-bit target even though it was never
+   implemented in hardware.  The software implementation only needs 8-byte
+   alignment.  This is to match the HP compilers.  */
 #define BIGGEST_ALIGNMENT (2 * BITS_PER_WORD)
 
+/* Alignment, in bits, a C conformant malloc implementation has to provide.
+   The HP-UX malloc implementation provides a default alignment of 8 bytes.
+   This can be increased with mallopt.  The glibc implementation also provides
+   8-byte alignment.  Note that this isn't enough for various POSIX types such
+   as pthread_mutex_t.  However, since we no longer need the 16-byte alignment
+   for atomic operations, we ignore the nominal alignment specified for these
+   types.  The same is true for long double on 64-bit HP-UX.  */
+#define MALLOC_ABI_ALIGNMENT (64)
+
 /* Get around hp-ux assembler bug, and make strcpy of constants fast.  */
 #define CONSTANT_ALIGNMENT(EXP, ALIGN)		\
   (TREE_CODE (EXP) == STRING_CST		\
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 240688)
+++ cp/decl.c	(working copy)
@@ -4141,7 +4141,7 @@
   if (aligned_new_threshold == -1)
     aligned_new_threshold = (cxx_dialect >= cxx1z) ? 1 : 0;
   if (aligned_new_threshold == 1)
-    aligned_new_threshold = max_align_t_align () / BITS_PER_UNIT;
+    aligned_new_threshold = targetm.max_align_t_align () / BITS_PER_UNIT;
 
   {
     tree newattrs, extvisattr;
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 240688)
+++ cp/init.c	(working copy)
@@ -3017,7 +3017,7 @@
   gcc_assert (alloc_fn != NULL_TREE);
 
   if (warn_aligned_new
-      && TYPE_ALIGN (elt_type) > max_align_t_align ()
+      && TYPE_ALIGN (elt_type) > targetm.max_align_t_align ()
       && (warn_aligned_new > 1
 	  || CP_DECL_CONTEXT (alloc_fn) == global_namespace)
       && !aligned_allocation_fn_p (alloc_fn))
Index: doc/tm.texi
===================================================================
--- doc/tm.texi	(revision 240688)
+++ doc/tm.texi	(working copy)
@@ -11686,6 +11686,10 @@
 ISO C11 requires atomic compound assignments that may raise floating-point exceptions to raise exceptions corresponding to the arithmetic operation whose result was successfully stored in a compare-and-exchange sequence.  This requires code equivalent to calls to @code{feholdexcept}, @code{feclearexcept} and @code{feupdateenv} to be generated at appropriate points in the compare-and-exchange sequence.  This hook should set @code{*@var{hold}} to an expression equivalent to the call to @code{feholdexcept}, @code{*@var{clear}} to an expression equivalent to the call to @code{feclearexcept} and @code{*@var{update}} to an expression equivalent to the call to @code{feupdateenv}.  The three expressions are @code{NULL_TREE} on entry to the hook and may be left as @code{NULL_TREE} if no code is required in a particular place.  The default implementation leaves all three expressions as @code{NULL_TREE}.  The @code{__atomic_feraiseexcept} function from @code{libatomic} may be of use as part of the code generated in @code{*@var{update}}.
 @end deftypefn
 
+@deftypefn {Target Hook} {unsigned int} TARGET_MAX_ALIGN_T_ALIGN (void)
+If defined, this function returns the alignment in bits for std:max_align_t. The default is suitable for most targets.  
+@end deftypefn
+
 @deftypefn {Target Hook} void TARGET_RECORD_OFFLOAD_SYMBOL (tree)
 Used when offloaded functions are seen in the compilation unit and no named
 sections are available.  It is called once for each symbol that must be
Index: doc/tm.texi.in
===================================================================
--- doc/tm.texi.in	(revision 240688)
+++ doc/tm.texi.in	(working copy)
@@ -8224,6 +8224,8 @@
 
 @hook TARGET_ATOMIC_ASSIGN_EXPAND_FENV
 
+@hook TARGET_MAX_ALIGN_T_ALIGN
+
 @hook TARGET_RECORD_OFFLOAD_SYMBOL
 
 @hook TARGET_OFFLOAD_OPTIONS
Index: target.def
===================================================================
--- target.def	(revision 240688)
+++ target.def	(working copy)
@@ -5894,6 +5894,15 @@
  void, (tree *hold, tree *clear, tree *update),
  default_atomic_assign_expand_fenv)
 
+/* Return an unsigned int representing the alignment (in bits) of
+   std:max_align_t.  This allows the default alignment to be overridden.  */
+DEFHOOK
+(max_align_t_align,
+"If defined, this function returns the alignment in bits for std:max_align_t.\
+ The default is suitable for most targets.  ",
+ unsigned int, (void),
+ default_max_align_t_align)
+
 /* Leave the boolean fields at the end.  */
 
 /* True if we can create zeroed data by switching to a BSS section
Index: targhooks.c
===================================================================
--- targhooks.c	(revision 240688)
+++ targhooks.c	(working copy)
@@ -1907,6 +1907,23 @@
 {
 }
 
+/* Default implementation of TARGET_MAX_ALIGN_T_ALIGN to return alignment
+   in bits of std::max_align_t.
+
+   [support.types.layout] The type max_align_t is a POD type whose alignment
+   requirement is at least as great as that of every scalar type, and whose
+   alignment requirement is supported in every context.  */
+
+unsigned int
+default_max_align_t_align (void)
+{
+  unsigned int max_align = MAX (TYPE_ALIGN (long_long_integer_type_node),
+                                TYPE_ALIGN (long_double_type_node));
+  if (float128_type_node != NULL_TREE)
+    max_align = MAX (max_align, TYPE_ALIGN (float128_type_node));
+  return max_align;
+}
+
 #ifndef PAD_VARARGS_DOWN
 #define PAD_VARARGS_DOWN BYTES_BIG_ENDIAN
 #endif
Index: targhooks.h
===================================================================
--- targhooks.h	(revision 240688)
+++ targhooks.h	(working copy)
@@ -262,5 +262,6 @@
 				       optimization_type);
 
 extern unsigned int default_max_noce_ifcvt_seq_cost (edge);
+extern unsigned int default_max_align_t_align (void);
 
 #endif /* GCC_TARGHOOKS_H */

^ permalink raw reply	[flat|nested] 34+ messages in thread
end of thread, other threads:[~2017-02-25 22:56 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-08 16:43 [PATCH] Implement new hook for max_align_t_align Bernd Edlinger
  -- strict thread matches above, loose matches on Subject: below --
2016-10-08 17:01 Bernd Edlinger
2016-10-08 17:36 ` John David Anglin
2016-10-09  8:35   ` Bernd Edlinger
2016-10-09 17:52     ` John David Anglin
2016-10-10 18:21       ` John David Anglin
2016-10-11 18:51         ` Jason Merrill
2016-10-11 18:59           ` DJ Delorie
2016-10-11 20:12             ` Jason Merrill
2016-10-11 20:55               ` John David Anglin
2016-10-11 20:57                 ` Jakub Jelinek
2016-10-11 21:27                 ` Jason Merrill
2016-10-11 20:04           ` John David Anglin
2016-10-12  7:02             ` Carlos O'Donell
2016-10-12  7:25               ` Jakub Jelinek
2016-10-12  7:52                 ` Florian Weimer
2016-10-12  8:02                   ` Jakub Jelinek
2016-10-12 12:13                     ` John David Anglin
2016-10-12 12:33                       ` Bernd Schmidt
2016-10-12 12:43                         ` Richard Biener
2016-10-12 12:46                           ` Bernd Schmidt
2016-10-12 15:51                           ` Joseph Myers
2016-10-12 13:48                     ` Jason Merrill
2016-10-12 14:17                       ` John David Anglin
2016-10-12 19:59                         ` Carlos O'Donell
2016-10-12 16:14                     ` Jeff Law
2016-10-12 17:24                       ` John David Anglin
2017-02-25 17:46                         ` Gerald Pfeifer
2017-02-25 22:13                           ` John David Anglin
2017-02-25 22:46                             ` Gerald Pfeifer
2017-02-25 23:46                               ` John David Anglin
2016-10-12 18:01                       ` Florian Weimer
2016-10-12 18:13                         ` John David Anglin
2016-10-08 15:45 John David Anglin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).