Re: [PATCH 1/4][Ada,DJGPP] Ada support for DJGPP

2016-08-13 Thread Arnaud Charlet

>>>  * ada/adaint.c (__gnat_is_djgpp): define (1 for DJGPP host, 0
>>> otherwise). * ada/s-os_lib.ads (Is_Djgpp): import __gnat_is_djgpp as
>>> constant. * ada/s-os_lib.adb (Normalize_Pathname): support DJGPP special
>>> paths (/dev/*) for DJGPP hosts
>> The patch does more than this though:
> Updated ChangeLog entry:
> 
> Subject: [PATCH 1/4] [DJGPP, Ada] File path handling for DJGPP host
> 
> * ada/adaint.c (__gnat_is_djgpp): define (1 for DJGPP host, 0 otherwise).
> * ada/s-os_lib.ads (Is_Djgpp): import __gnat_is_djgpp as constant.
> * ada/s-os_lib.adb (Normalize_Pathname): support DJGPP special paths (/dev/*) 
> for DJGPP hosts,
> (Normalize_Pathname): do not convert '/' to '\' for DJGPP hosts.
> 
> 
>> 
>> @@ -2242,8 +2271,11 @@ package body System.OS_Lib is
>>end File_Name_Conversion;
>>  --  Replace all '/' by Directory Separators (this is for Windows)
>> +  --  No need to do that however for DJGPP
>>  -  if Directory_Separator /= '/' then
>> +  if Directory_Separator /= '/'
>> +and then Is_Djgpp = 0
>> +  then
>>   for Index in 1 .. End_Path loop
>>  if Path_Buffer (Index) = '/' then
>> Path_Buffer (Index) := Directory_Separator;
>> 
>> Why does DJGPP need to be special-cased here?  In order to disable some
>> further transformation downstream?  Could DIR_SEPARATOR be just '/'?
>> 
> Both '/' and '\' must be supported as directory separators. So 
> DIR_SEPARATOR='/' is not OK in this case.
> 
> Unconditional converting '/' to '\' in case of DJGPP native build causes 
> gnatmake to break. Retested it today it with gcc-6.1.0. The problem is that 
> special directory name /dev/env/DJDIR is used as prefix for DJGPP (it 
> resolves to $DJDIR in execution time)

This part of the patch is really too kludgy and too intrusive, you will need to 
find find a less intrusive way to address this djgpp special case.


Arno


Re: [PATCH] Add mark_spam.py script

2016-08-13 Thread Martin Liška

On 08/12/2016 11:15 PM, Joseph Myers wrote:

Next observation on this script: it dies if a bug number in the given
range doesn't exist, with an error like:

Marking as spam: PR75336
Traceback (most recent call last):
  File "./mark_spam.py", line 98, in 
mark_as_spam(id, args.api_key, args.verbose)
  File "./mark_spam.py", line 38, in mark_as_spam
cc_list = response['bugs'][0]['cc']
KeyError: 'bugs'

It would be more convenient if it ignored nonexistent bugs rather than
falling over like this, so that it's only necessary to check that the
range you pass to the script has no non-spam bugs in it, not that every
bug number in the range exists.


That's easy to solve, please try to apply following patch on top
of current trunk.



(I don't know why there are gaps in the bug numbers; I suppose some error
/ timeout occurred while the spammers were creating bugs, at a point after
a bug number had been reserved in the database but before the transaction
creating the bug was complete - in such circumstances, databases don't
necessarily unwind the reservation of a number.)

diff --git a/contrib/mark_spam.py b/contrib/mark_spam.py
index 569a03d..f206356 100755
--- a/contrib/mark_spam.py
+++ b/contrib/mark_spam.py
@@ -34,6 +34,10 @@ def mark_as_spam(id, api_key, verbose):
 r = requests.get(u)
 response = json.loads(r.text)
 
+if 'error' in response and response['error']:
+print(response['message'])
+return
+
 # 2) mark the bug as spam
 cc_list = response['bugs'][0]['cc']
 data = {
@@ -49,6 +53,7 @@ def mark_as_spam(id, api_key, verbose):
 'cc': {'remove': cc_list},
 'priority': 'P5',
 'severity': 'trivial',
+'url': '',
 'assigned_to': 'unassig...@gcc.gnu.org' }
 
 r = requests.put(u, json = data)
@@ -74,7 +79,12 @@ def mark_as_spam(id, api_key, verbose):
 for a in attachments:
 attachment_id = a['id']
 url = '%sbug/attachment/%d' % (base_url, attachment_id)
-r = requests.put(url, json = {'ids': [attachment_id], 'summary': 'spam', 'comment': 'spam', 'is_obsolete': True, 'api_key': api_key})
+r = requests.put(url, json = {'ids': [attachment_id],
+'summary': 'spam',
+'file_name': 'spam',
+'content_type': 'application/x-spam',
+'is_obsolete': True,
+'api_key': api_key})
 if verbose:
 print(r)
 print(r.text)


Re: [PATCH] New hpux fix to add noreturn attribute to longjmp declarations in setjmp.h

2016-08-13 Thread Bruce Korb
Looks good to me.

On Sat, Aug 13, 2016 at 9:55 AM, John David Anglin  wrote:
> Currently, trunk fails to boot fortran on hpux because of the following error:
>
> /xxx/gnu/gcc/objdir/./prev-gcc/xg++ -B/xxx/gnu/gcc/objdir/./prev-gcc/ 
> -B/opt/gnu64/gcc/gcc-7/hppa64-hp-hpux11.00/bin/ -nostdinc++ 
> -B/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/src/.libs 
> -B/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/libsupc++/.libs  
> -I/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11
> .00/libstdc++-v3/include/hppa64-hp-hpux11.00  
> -I/xxx/gnu/gcc/objdir/prev-hppa64-
> hp-hpux11.00/libstdc++-v3/include  -I/xxx/gnu/gcc/gcc/libstdc++-v3/libsupc++ 
> -L/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/src/.libs 
> -L/xxx/gnu/gc
> c/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/libsupc++/.libs -fno-PIE -c  
> -DIN
> _GCC_FRONTEND -g -O2 -DIN_GCC -fno-exceptions -fno-rtti 
> -fasynchronous-unwin
> d-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual 
> -Wmissing-format-at
> tribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros 
> -Wno-
> overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -Ifortran 
> -I../../gc
> c/gcc -I../../gcc/gcc/fortran -I../../gcc/gcc/../include 
> -I../../gcc/gcc/../libc
> pp/include -I/opt/gnu64/gcc/gmp/include  -I../../gcc/gcc/../libdecnumber 
> -I../..
> /gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace
>  -o fortran/parse.o -MT fortran/parse.o -MMD -MP -MF fortran/.deps/parse.TPo 
> ../
> ../gcc/gcc/fortran/parse.c
> ../../gcc/gcc/fortran/parse.c: In function 'void unexpected_eof()':
> ../../gcc/gcc/fortran/parse.c:2618:1: error: 'noreturn' function does return 
> [-W
> error]
>
> The attached patch fixes the above by adding the noreturn attribute to the 
> "longjmp" declarations in setjmp.h.
>
> Okay for trunk?
>
> Dave
> --
> John David Anglin   dave.ang...@bell.net
>


[PATCH] New hpux fix to add noreturn attribute to longjmp declarations in setjmp.h

2016-08-13 Thread John David Anglin
Currently, trunk fails to boot fortran on hpux because of the following error:

/xxx/gnu/gcc/objdir/./prev-gcc/xg++ -B/xxx/gnu/gcc/objdir/./prev-gcc/ 
-B/opt/gnu64/gcc/gcc-7/hppa64-hp-hpux11.00/bin/ -nostdinc++ 
-B/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/src/.libs 
-B/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/libsupc++/.libs  
-I/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11
.00/libstdc++-v3/include/hppa64-hp-hpux11.00  -I/xxx/gnu/gcc/objdir/prev-hppa64-
hp-hpux11.00/libstdc++-v3/include  -I/xxx/gnu/gcc/gcc/libstdc++-v3/libsupc++ 
-L/xxx/gnu/gcc/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/src/.libs 
-L/xxx/gnu/gc
c/objdir/prev-hppa64-hp-hpux11.00/libstdc++-v3/libsupc++/.libs -fno-PIE -c  -DIN
_GCC_FRONTEND -g -O2 -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwin
d-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-at
tribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-
overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -Ifortran -I../../gc
c/gcc -I../../gcc/gcc/fortran -I../../gcc/gcc/../include -I../../gcc/gcc/../libc
pp/include -I/opt/gnu64/gcc/gmp/include  -I../../gcc/gcc/../libdecnumber -I../..
/gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace  
 -o fortran/parse.o -MT fortran/parse.o -MMD -MP -MF fortran/.deps/parse.TPo ../
../gcc/gcc/fortran/parse.c
../../gcc/gcc/fortran/parse.c: In function 'void unexpected_eof()':
../../gcc/gcc/fortran/parse.c:2618:1: error: 'noreturn' function does return [-W
error]

The attached patch fixes the above by adding the noreturn attribute to the 
"longjmp" declarations in setjmp.h.

Okay for trunk?

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

2016-08-13  John David Anglin  

* inclhack.def (hpux_longjmp): New fix.
* fixincl.x: Regenerate.
* tests/base/setjmp.h: New test file.

Index: inclhack.def
===
--- inclhack.def(revision 239324)
+++ inclhack.def(working copy)
@@ -2642,6 +2642,21 @@
 };
 
 /*
+ *  Add noreturn attribute to longjmp declarations in hpux 
+ */
+fix = {
+hackname = hpux_longjmp;
+mach = "*-hp-hpux*";
+files= setjmp.h;
+select   = "^[ \t]*extern[ \t]+void.*longjmp[ \t]+__\\(\\(.*int\\)\\)";
+
+c_fix = format;
+c_fix_arg = "%0 __attribute__ ((__noreturn__))";
+
+test_text = 'extern void   longjmp __((jmp_buf, int));';
+};
+
+/*
  *  Fix hpux10.20  to avoid invalid forward decl
  */
 fix = {
--- /dev/null   2016-08-13 12:36:13 -0400
+++ tests/base/setjmp.h 2016-08-10 09:41:48 -0400
@@ -0,0 +1,14 @@
+/*  DO NOT EDIT THIS FILE.
+
+It has been auto-edited by fixincludes from:
+
+   "fixinc/tests/inc/setjmp.h"
+
+This had to be done to correct non-standard usages in the
+original, manufacturer supplied header file.  */
+
+
+
+#if defined( HPUX_LONGJMP_CHECK )
+extern voidlongjmp __((jmp_buf, int)) __attribute__ ((__noreturn__));
+#endif  /* HPUX_LONGJMP_CHECK */


[BUILDROBOT] avr broken (was: [PATCH 1/4] Cherry-pick fprofile-generate-atomic from google/gcc-4_9 branch)

2016-08-13 Thread Jan-Benedict Glaw
On Fri, 2016-08-05 15:43:02 +0200, Martin Liška  wrote:
[...]
> Great, attaching install candidate.

> >From 0b3ac8636ef34b02e301f22c86dde0602f9969ef Mon Sep 17 00:00:00 2001
> From: marxin 
> Date: Thu, 28 Jul 2016 14:32:47 +0200
> Subject: [PATCH 1/4] Cherry-pick fprofile-generate-atomic from google/gcc-4_9
>  branch
> 
> gcc/ChangeLog:
> 
> 2016-08-05  Martin Liska  
> 
>   Cherry picked (and modified) from google-4_7 branch
>   2012-12-26  Rong Xu  
>   * common.opt (fprofile-update): Add new flag.
>   * coretypes.h: Define enum profile_update.
>   * doc/invoke.texi: Document -fprofile-update.
>   * gcov-io.h: Declare GCOV_TYPE_ATOMIC_FETCH_ADD and
>   GCOV_TYPE_ATOMIC_FETCH_ADD_FN.
>   * tree-profile.c (gimple_init_edge_profiler): Generate
>   also atomic profiler update.
>   (gimple_gen_edge_profiler): Likewise.
[...]
> --- a/gcc/gcov-io.h
> +++ b/gcc/gcov-io.h
> @@ -164,6 +164,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively. 
>  If not, see
>  #ifndef GCC_GCOV_IO_H
>  #define GCC_GCOV_IO_H
>  
> +#if LONG_LONG_TYPE_SIZE > 32
> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_8
> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_8
> +#else
> +#define GCOV_TYPE_ATOMIC_FETCH_ADD_FN __atomic_fetch_add_4
> +#define GCOV_TYPE_ATOMIC_FETCH_ADD BUILT_IN_ATOMIC_FETCH_ADD_4
> +#endif
> +
>  #ifndef IN_LIBGCOV
>  /* About the host */
>  

This doesn't work for AVR since their LONG_LONG_TYPE_SIZE depents on
target flags (see eg. build
http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=602648)



g++ -fno-PIE -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE   
-fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing 
-Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual 
-pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings 
-fno-common  -DHAVE_CONFIG_H -I. -I. -I/home/jbglaw/repos/gcc/gcc 
-I/home/jbglaw/repos/gcc/gcc/. -I/home/jbglaw/repos/gcc/gcc/../include 
-I/home/jbglaw/repos/gcc/gcc/../libcpp/include  
-I/home/jbglaw/repos/gcc/gcc/../libdecnumber 
-I/home/jbglaw/repos/gcc/gcc/../libdecnumber/dpd -I../libdecnumber 
-I/home/jbglaw/repos/gcc/gcc/../libbacktrace   -o auto-profile.o -MT 
auto-profile.o -MMD -MP -MF ./.deps/auto-profile.TPo 
/home/jbglaw/repos/gcc/gcc/auto-profile.c
In file included from ./tm.h:19:0,
 from /home/jbglaw/repos/gcc/gcc/backend.h:28,
 from /home/jbglaw/repos/gcc/gcc/auto-profile.c:26:
./options.h:261:36: error: token "." is not valid in preprocessor expressions
 #define target_flags global_options.x_target_flags
^
./options.h:5153:23: note: in expansion of macro ‘target_flags’
 #define TARGET_INT8 ((target_flags & MASK_INT8) != 0)
   ^
/home/jbglaw/repos/gcc/gcc/config/avr/avr.h:138:24: note: in expansion of macro 
‘TARGET_INT8’
 #define INT_TYPE_SIZE (TARGET_INT8 ? 8 : 16)
^
/home/jbglaw/repos/gcc/gcc/config/avr/avr.h:141:30: note: in expansion of macro 
‘INT_TYPE_SIZE’
 #define LONG_LONG_TYPE_SIZE (INT_TYPE_SIZE == 8 ? 32 : 64)
  ^
/home/jbglaw/repos/gcc/gcc/gcov-io.h:167:5: note: in expansion of macro 
‘LONG_LONG_TYPE_SIZE’
 #if LONG_LONG_TYPE_SIZE > 32
 ^
Makefile:1096: recipe for target 'auto-profile.o' failed
make[1]: *** [auto-profile.o] Error 1
make[1]: Leaving directory '/home/jbglaw/build/avr/build-gcc/gcc'


MfG, JBG

-- 
  Jan-Benedict Glaw  jbg...@lug-owl.de  +49-172-7608481
Signature of:http://www.chiark.greenend.org.uk/~sgtatham/bugs.html
the second  :


signature.asc
Description: Digital signature


Re: RFC [1/2] divmod transform

2016-08-13 Thread Prathamesh Kulkarni
On 13 August 2016 at 16:56, Prathamesh Kulkarni
 wrote:
> On 28 July 2016 at 19:05, Prathamesh Kulkarni
>  wrote:
>> On 8 June 2016 at 19:53, Richard Biener  wrote:
>>> On Fri, 3 Jun 2016, Jim Wilson wrote:
>>>
 On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
 > Joseph - do you know sth about why there's not a full set of divmod
 > libfuncs in libgcc?

 Because udivmoddi4 isn't a libfunc, it is a helper function for the
 div and mov libfuncs.  Since we can compute the signed div and mod
 results from udivmoddi4, there was no need to also add a signed
 version of it.  It was given a libfunc style name so that we had the
 option of making it a libfunc in the future, but that never happened.
 There was no support for calling any divmod libfunc until it was added
 as a special case to call an ARM library (not libgcc) function.  This
 happened here

 2004-08-09  Mark Mitchell  

 * config.gcc (arm*-*-eabi*): New target.
 * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
 (TARGET_LIB_INT_CMP_BIASED): Likewise.
 * expmed.c (expand_divmod): Try a two-valued divmod function as a
 last resort.
 ...
 * config/arm/arm.c (arm_init_libfuncs): New function.
 (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
 (TARGET_INIT_LIBFUNCS): Define it.
 ...

 Later, two ports added their own divmod libfuncs, but I don't see any
 evidence that they were ever used, since there is no support for
 calling divmod other than the expand_divmod last resort code that only
 triggers for ARM.

 It is only now that Prathamesh is adding gimple support for divmod
 operations that we need to worry about getting this right, without
 breaking the existing ARM library support or the existing udivmoddi4
 support.
>>>
>>> Ok, so as he is primarily targeting the special arm divmod libcall
>>> I suppose we can live with special-casing libcall handling to
>>> udivmoddi3.  It would be nice to not lie about divmod availablilty
>>> as libcall though... - it looks like the libcall is also guarded
>>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
>>> like on x86).
>>>
>>> So not sure where to go from here.
>> Hi,
>> I have attached patch, which is rebased on trunk.
>> Needed to update divmod-7.c, which now gets transformed to divmod
>> thanks to your code-hoisting patch -;)
>> We still have the issue of optab_libfunc() returning non-existent
>> libcalls. As in previous patch, I am checking
>> explicitly for "__udivmoddi4", with a FIXME note.
>> I hope that's okay for now ?
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu,
>> armv8l-unknown-linux-gnueabihf.
>> Bootstrap+test in progress on i686-linux-gnu.
>> Cross-tested on arm*-*-*.
> Hi Richard,
> I have following two approaches to workaround optab_libfunc issue:
>
> a) Not lie about divmod libfunc availability by setting libcall entry to NULL
> for sdivmod_optab in optabs.def.
> Patch posted for that here:
> https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
> Although it doesn't cause any regressions with the gcc testsuite,
> I am not sure if this change is correct.
>
> b) Perform the transform only if target-specific divmod is available,
> ie, drop targeting
> __udivmoddi4. I have attached (untested) patch for that.
> When/If we have the optab_libfunc issue resolved, we can later target 
> "generic"
> divmod libfunc.
Oops, small mistake in the previous patch.
We also want to check if target has optab_libfunc set for the given mode.
Corrected in this version.

Thanks,
Prathamesh
>
> Do either of these approaches look reasonable ?
>
> PS: I am on vacation next week, will get back to working on patch
> after returning.
>
> Thanks,
> Prathamesh
>>
>> Thanks,
>> Prathamesh
>>>
>>> Richard.
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index f506a83..618c810 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2012,28 +2012,14 @@ default_max_noce_ifcvt_seq_cost (edge e)
DImode __udivmoddi4 (DImode op0, DImode op1, DImode *rem).  */
 
 void
-default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
-  rtx op0, rtx op1,
-  rtx *quot_p, rtx *rem_p)
+default_expand_divmod_libfunc (bool unsignedp ATTRIBUTE_UNUSED,
+  machine_mode mode ATTRIBUTE_UNUSED,
+  rtx op0 ATTRIBUTE_UNUSED,
+  rtx op1 ATTRIBUTE_UNUSED,
+   rtx *quot_p ATTRIBUTE_UNUSED,
+  rtx *rem_p ATTRIBUTE_UNUSED)
 {
-  gcc_assert (mode == DImode);
-  gcc_assert (unsignedp);
-
-  rtx libfunc = optab_libfunc (udivmod_optab, DImode);
-  gcc_assert (libfunc);

Re: [PATCH 1/4][Ada,DJGPP] Ada support for DJGPP

2016-08-13 Thread Andris Pavenis

On 08/12/2016 07:18 PM, Eric Botcazou wrote:

2016-07-30 Andris Pavenis 

  * ada/adaint.c (__gnat_is_djgpp): define (1 for DJGPP host, 0
otherwise). * ada/s-os_lib.ads (Is_Djgpp): import __gnat_is_djgpp as
constant. * ada/s-os_lib.adb (Normalize_Pathname): support DJGPP special
paths (/dev/*) for DJGPP hosts

The patch does more than this though:

Updated ChangeLog entry:

Subject: [PATCH 1/4] [DJGPP, Ada] File path handling for DJGPP host

* ada/adaint.c (__gnat_is_djgpp): define (1 for DJGPP host, 0 otherwise).
* ada/s-os_lib.ads (Is_Djgpp): import __gnat_is_djgpp as constant.
* ada/s-os_lib.adb (Normalize_Pathname): support DJGPP special paths (/dev/*) 
for DJGPP hosts,
 (Normalize_Pathname): do not convert '/' to '\' for DJGPP hosts.




@@ -2242,8 +2271,11 @@ package body System.OS_Lib is
end File_Name_Conversion;
  
--  Replace all '/' by Directory Separators (this is for Windows)

+  --  No need to do that however for DJGPP
  
-  if Directory_Separator /= '/' then

+  if Directory_Separator /= '/'
+and then Is_Djgpp = 0
+  then
   for Index in 1 .. End_Path loop
  if Path_Buffer (Index) = '/' then
 Path_Buffer (Index) := Directory_Separator;

Why does DJGPP need to be special-cased here?  In order to disable some
further transformation downstream?  Could DIR_SEPARATOR be just '/'?

Both '/' and '\' must be supported as directory separators. So DIR_SEPARATOR='/' is not OK in this 
case.


Unconditional converting '/' to '\' in case of DJGPP native build causes gnatmake to break. 
Retested it today it with gcc-6.1.0. The problem is that special directory name /dev/env/DJDIR is 
used as prefix for DJGPP (it resolves to $DJDIR in execution time)


Andris

PS. Maybe it could be nicer if Normalize_Pathname would be in a separate package. This way one 
could override it for all targets for which special handling is needed.








Re: RFC [1/2] divmod transform

2016-08-13 Thread Prathamesh Kulkarni
On 28 July 2016 at 19:05, Prathamesh Kulkarni
 wrote:
> On 8 June 2016 at 19:53, Richard Biener  wrote:
>> On Fri, 3 Jun 2016, Jim Wilson wrote:
>>
>>> On Mon, May 30, 2016 at 12:45 AM, Richard Biener  wrote:
>>> > Joseph - do you know sth about why there's not a full set of divmod
>>> > libfuncs in libgcc?
>>>
>>> Because udivmoddi4 isn't a libfunc, it is a helper function for the
>>> div and mov libfuncs.  Since we can compute the signed div and mod
>>> results from udivmoddi4, there was no need to also add a signed
>>> version of it.  It was given a libfunc style name so that we had the
>>> option of making it a libfunc in the future, but that never happened.
>>> There was no support for calling any divmod libfunc until it was added
>>> as a special case to call an ARM library (not libgcc) function.  This
>>> happened here
>>>
>>> 2004-08-09  Mark Mitchell  
>>>
>>> * config.gcc (arm*-*-eabi*): New target.
>>> * defaults.h (TARGET_LIBGCC_FUNCS): New macro.
>>> (TARGET_LIB_INT_CMP_BIASED): Likewise.
>>> * expmed.c (expand_divmod): Try a two-valued divmod function as a
>>> last resort.
>>> ...
>>> * config/arm/arm.c (arm_init_libfuncs): New function.
>>> (arm_compute_initial_eliminatino_offset): Return HOST_WIDE_INT.
>>> (TARGET_INIT_LIBFUNCS): Define it.
>>> ...
>>>
>>> Later, two ports added their own divmod libfuncs, but I don't see any
>>> evidence that they were ever used, since there is no support for
>>> calling divmod other than the expand_divmod last resort code that only
>>> triggers for ARM.
>>>
>>> It is only now that Prathamesh is adding gimple support for divmod
>>> operations that we need to worry about getting this right, without
>>> breaking the existing ARM library support or the existing udivmoddi4
>>> support.
>>
>> Ok, so as he is primarily targeting the special arm divmod libcall
>> I suppose we can live with special-casing libcall handling to
>> udivmoddi3.  It would be nice to not lie about divmod availablilty
>> as libcall though... - it looks like the libcall is also guarded
>> on TARGET_HAS_NO_HW_DIVIDE (unless it was available historically
>> like on x86).
>>
>> So not sure where to go from here.
> Hi,
> I have attached patch, which is rebased on trunk.
> Needed to update divmod-7.c, which now gets transformed to divmod
> thanks to your code-hoisting patch -;)
> We still have the issue of optab_libfunc() returning non-existent
> libcalls. As in previous patch, I am checking
> explicitly for "__udivmoddi4", with a FIXME note.
> I hope that's okay for now ?
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu,
> armv8l-unknown-linux-gnueabihf.
> Bootstrap+test in progress on i686-linux-gnu.
> Cross-tested on arm*-*-*.
Hi Richard,
I have following two approaches to workaround optab_libfunc issue:

a) Not lie about divmod libfunc availability by setting libcall entry to NULL
for sdivmod_optab in optabs.def.
Patch posted for that here:
https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01015.html
Although it doesn't cause any regressions with the gcc testsuite,
I am not sure if this change is correct.

b) Perform the transform only if target-specific divmod is available,
ie, drop targeting
__udivmoddi4. I have attached (untested) patch for that.
When/If we have the optab_libfunc issue resolved, we can later target "generic"
divmod libfunc.

Do either of these approaches look reasonable ?

PS: I am on vacation next week, will get back to working on patch
after returning.

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
>>
>> Richard.
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index f506a83..618c810 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -2012,28 +2012,14 @@ default_max_noce_ifcvt_seq_cost (edge e)
DImode __udivmoddi4 (DImode op0, DImode op1, DImode *rem).  */
 
 void
-default_expand_divmod_libfunc (bool unsignedp, machine_mode mode,
-  rtx op0, rtx op1,
-  rtx *quot_p, rtx *rem_p)
+default_expand_divmod_libfunc (bool unsignedp ATTRIBUTE_UNUSED,
+  machine_mode mode ATTRIBUTE_UNUSED,
+  rtx op0 ATTRIBUTE_UNUSED,
+  rtx op1 ATTRIBUTE_UNUSED,
+   rtx *quot_p ATTRIBUTE_UNUSED,
+  rtx *rem_p ATTRIBUTE_UNUSED)
 {
-  gcc_assert (mode == DImode);
-  gcc_assert (unsignedp);
-
-  rtx libfunc = optab_libfunc (udivmod_optab, DImode);
-  gcc_assert (libfunc);
-  gcc_assert (!strcmp (XSTR (libfunc, 0), "__udivmoddi4"));
-
-  rtx remainder = assign_stack_temp (DImode, GET_MODE_SIZE (DImode));
-  rtx address = XEXP (remainder, 0);
-
-  rtx quotient = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST,
- DImode, 3,
- op0, GET_MODE (op0),
-