Re: [PATCH] libstdc++: Make it possible to annotate the shared pointer operations in the std::thread implementation
On Fri, Dec 23, 2011 at 5:04 PM, Paolo Carlini wrote: > First, do you have already a Copyright assignment on file? It's a > precondition for any non trivial contribution. I've just started the procedure to get a copyright assignment form filed. > That said, please leave alone the baselines. Otherwise, Jon can comment on > whether the reshuffling makes > sense and would be safe from the Abi point of view. I'll repost the patch on the libstdc++ mailing list as Jon asked. Bart.
Re: [BFIN] Hookize REGISTER_MOVE_COST and MEMORY_MOVE_COST
Hi.
Hi Anatoly,
I cannot apply your patch to a lean tree. I tried to save your email
as a text file, copy from thunderbird, copy from gmail, copy from the
mailing list archive. But neither works.
Regards,
Jie
You can use a patch now?
Index: gcc/config/bfin/bfin-protos.h
===
--- gcc/config/bfin/bfin-protos.h (revision 182658)
+++ gcc/config/bfin/bfin-protos.h (working copy)
@@ -85,9 +85,6 @@ extern bool bfin_longcall_p (rtx, int);
extern bool bfin_dsp_memref_p (rtx);
extern bool bfin_expand_movmem (rtx, rtx, rtx, rtx);
-extern int bfin_register_move_cost (enum machine_mode, enum reg_class,
- enum reg_class);
-extern int bfin_memory_move_cost (enum machine_mode, enum reg_class, int in);
extern enum reg_class secondary_input_reload_class (enum reg_class,
enum machine_mode,
rtx);
Index: gcc/config/bfin/bfin.c
===
--- gcc/config/bfin/bfin.c (revision 182658)
+++ gcc/config/bfin/bfin.c (working copy)
@@ -2149,12 +2149,11 @@ bfin_vector_mode_supported_p (enum machi
return mode == V2HImode;
}
-/* Return the cost of moving data from a register in class CLASS1 to
- one in class CLASS2. A cost of 2 is the default. */
+/* Worker function for TARGET_REGISTER_MOVE_COST. */
-int
+static int
bfin_register_move_cost (enum machine_mode mode,
-enum reg_class class1, enum reg_class class2)
+reg_class_t class1, reg_class_t class2)
{
/* These need secondary reloads, so they're more expensive. */
if ((class1 == CCREGS && !reg_class_subset_p (class2, DREGS))
@@ -2177,18 +2176,16 @@ bfin_register_move_cost (enum machine_mo
return 2;
}
-/* Return the cost of moving data of mode M between a
- register and memory. A value of 2 is the default; this cost is
- relative to those in `REGISTER_MOVE_COST'.
+/* Worker function for TARGET_MEMORY_MOVE_COST.
??? In theory L1 memory has single-cycle latency. We should add a switch
that tells the compiler whether we expect to use only L1 memory for the
program; it'll make the costs more accurate. */
-int
+static int
bfin_memory_move_cost (enum machine_mode mode ATTRIBUTE_UNUSED,
- enum reg_class rclass,
- int in ATTRIBUTE_UNUSED)
+ reg_class_t rclass,
+ bool in ATTRIBUTE_UNUSED)
{
/* Make memory accesses slightly more expensive than any register-register
move. Also, penalize non-DP registers, since they need secondary
@@ -5703,6 +5700,12 @@ bfin_conditional_register_usage (void)
#undef TARGET_ADDRESS_COST
#define TARGET_ADDRESS_COST bfin_address_cost
+#undef TARGET_REGISTER_MOVE_COST
+#define TARGET_REGISTER_MOVE_COST bfin_register_move_cost
+
+#undef TARGET_MEMORY_MOVE_COST
+#define TARGET_MEMORY_MOVE_COST bfin_memory_move_cost
+
#undef TARGET_ASM_INTEGER
#define TARGET_ASM_INTEGER bfin_assemble_integer
Index: gcc/config/bfin/bfin.h
===
--- gcc/config/bfin/bfin.h (revision 182658)
+++ gcc/config/bfin/bfin.h (working copy)
@@ -975,29 +975,6 @@ typedef struct {
/* Do not put function addr into constant pool */
#define NO_FUNCTION_CSE 1
-/* A C expression for the cost of moving data from a register in class FROM to
- one in class TO. The classes are expressed using the enumeration values
- such as `GENERAL_REGS'. A value of 2 is the default; other values are
- interpreted relative to that.
-
- It is not required that the cost always equal 2 when FROM is the same as TO;
- on some machines it is expensive to move between registers if they are not
- general registers. */
-
-#define REGISTER_MOVE_COST(MODE, CLASS1, CLASS2) \
- bfin_register_move_cost ((MODE), (CLASS1), (CLASS2))
-
-/* A C expression for the cost of moving data of mode M between a
- register and memory. A value of 2 is the default; this cost is
- relative to those in `REGISTER_MOVE_COST'.
-
- If moving between registers and memory is more expensive than
- between two registers, you should define this macro to express the
- relative cost. */
-
-#define MEMORY_MOVE_COST(MODE, CLASS, IN) \
- bfin_memory_move_cost ((MODE), (CLASS), (IN))
-
/* Specify the machine mode that this machine uses
for the index in the tablejump instruction. */
#define CASE_VECTOR_MODE SImode
Anatoly.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On 24 December 2011 09:53, Bart Van Assche wrote: > As documented in the libstdc++ manual, the shared pointer operations in > libstdc++ headers can be instrumented by defining the macros > _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE()/AFTER() and either libstdc++ or > thread.cc has to be rebuilt in order to instrument the remaining shared > pointer operations. However, rebuilding is inconvenient. So let's move the > thread wrapper code from thread.cc into . The patch is not OK because the function passed to pthread_create must be extern "C" and a static member function is not extern "C" (G++ fails to diagnose that error, but that's a long-standing bug: http://gcc.gnu.org/PR2316 ) Apart from that, I prefer to keep the thread-launching logic in the library rather than headers, as it allows us to change it more easily and for users to benefit just by linking to a newer library rather than recompiling. I don't think the data race detection macros are a compelling reason to change that. As I noted yesterday on PR 51504, you don't need to recompile the whole library, you only need to recompile thread.cc with the data race detection macros defined and link that object earlier in your linker search path than libstdc++ to allow symbol interposition to replace std::thread::_M_start_thread from the library with your instrument version of that symbol. I'll update the manual to say that.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On 24 December 2011 11:11, Jonathan Wakely wrote: > > Apart from that, I prefer to keep the thread-launching logic in the > library rather than headers, as it allows us to change it more easily > and for users to benefit just by linking to a newer library rather > than recompiling. I don't think the data race detection macros are a > compelling reason to change that. An example improvement (which I'm working on) is to give better stack traces when a process is terminated by an uncaught exception in a function run by std::thread. If that's improved in the library, all existing code using std::thread would get better stack traces just by linking to a newer libstdc++.so, rather than having to recompile with a newer GCC.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On Sat, Dec 24, 2011 at 11:15 AM, Jonathan Wakely wrote: > On 24 December 2011 11:11, Jonathan Wakely wrote: >> Apart from that, I prefer to keep the thread-launching logic in the >> library rather than headers, as it allows us to change it more easily >> and for users to benefit just by linking to a newer library rather >> than recompiling. I don't think the data race detection macros are a >> compelling reason to change that. > > An example improvement (which I'm working on) is to give better stack > traces when a process is terminated by an uncaught exception in a > function run by std::thread. If that's improved in the library, all > existing code using std::thread would get better stack traces just by > linking to a newer libstdc++.so, rather than having to recompile with > a newer GCC. Would it be acceptable to replace the _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE() and ..._AFTER() macros by something like this: if (pf) (*pf)(addr, before_or_after_flag) where pf is a function pointer that is NULL by default and can be set by libstdc++ users. That approach would also avoid that libstdc++ has to be recompiled in order to instrument reference count decrementing for data race detectors. Bart.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On 24 December 2011 11:46, Bart Van Assche wrote: > > Would it be acceptable to replace the > _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE() and ..._AFTER() macros by > something like this: > > if (pf) (*pf)(addr, before_or_after_flag) > > where pf is a function pointer that is NULL by default and can be set > by libstdc++ users. That approach would also avoid that libstdc++ has > to be recompiled in order to instrument reference count decrementing > for data race detectors. I think the cost of checking that on every ref-count update would be too high for the vast majority of users who don't use race detectors. A macro has zero cost if it's not used, and even people using race detectors don't want them enabled all the time. As I said, you don't need to recompile the whole library to instrument the parts that are affected, only specific objects. I think the right thing to do is identify the parts that need to be recompiled and document how to do just that.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On Sat, Dec 24, 2011 at 12:05 PM, Jonathan Wakely wrote: > On 24 December 2011 11:46, Bart Van Assche wrote: >> Would it be acceptable to replace the >> _GLIBCXX_SYNCHRONIZATION_HAPPENS_BEFORE() and ..._AFTER() macros by >> something like this: >> >> if (pf) (*pf)(addr, before_or_after_flag) >> >> where pf is a function pointer that is NULL by default and can be set >> by libstdc++ users. That approach would also avoid that libstdc++ has >> to be recompiled in order to instrument reference count decrementing >> for data race detectors. > > I think the cost of checking that on every ref-count update would be > too high for the vast majority of users who don't use race detectors. > A macro has zero cost if it's not used, and even people using race > detectors don't want them enabled all the time. As I said, you don't > need to recompile the whole library to instrument the parts that are > affected, only specific objects. I think the right thing to do is > identify the parts that need to be recompiled and document how to do > just that. Would it be technically possible to implement a binary patching approach in a user space shared library similar to the binary patching technique used in the kprobe implementation in the Linux kernel ? For more information, see also http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=Documentation/kprobes.txt. Bart. Bart.
Re: [PATCH] Make it possible to annotate the shared pointer operations in the std::thread implementation
On 24 December 2011 12:14, Bart Van Assche wrote:
>
> Would it be technically possible to implement a binary patching
> approach in a user space shared library similar to the binary patching
> technique used in the kprobe implementation in the Linux kernel ?
>
> For more information, see also
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=blob;f=Documentation/kprobes.txt.
I don't know, but honestly, IMHO the use case of race detectors is not
important enough to bother. Recompiling the required objects and
linking to them is just not that difficult.
I think the guideline of "you don't pay for what you don't use" must
apply to any changes made for race detection in the library. While
runtime cost is the most important, maintenance cost also matters. We
accepted the patch adding the macros to the library sources because
they add no cost for most users and allow those who care to use race
detectors (albeit with some minor hassle of recompiling some source
files). You are now trying to turn something where we said "sure, why
not, let's include it" into something far more intrusive that is not
stritly necessary (just more convenient) and of no value for most
users.
I would support adding a configure option such as
--enable-libstdcxx-race-detection={drd,tsan,...} which automatically
built and installed a variant of the library using the specified race
detector, similar to the --enable-libstdcxx-debug option which builds
an unoptimized, unstripped libstdc++.so and places it in
${libdir}/debug/
One of the values for the option could be "user" which would make the
relevant libstdc++ files include a user-defined header, which could
define the macros to whatever you want, including calling a global
function pointer which may or may not be set.
Re: [patch] Flag-controlled type conversions/promotions
On Wed, Nov 09, 2011 at 06:09:58PM -0500, Andreas Kloeckner wrote: > Hi there, > > please find attached the patch and the Changelog entry for our work on > the fortran bug #48426. > > The attached patch implements the options > > -finteger-4-integer-8 > -freal-4-real-8 > -freal-4-real-10 > -freal-4-real-16 > -freal-8-real-4 > -freal-8-real-10 > -freal-8-real-16 > > to implement a variety of automatic type promotions. (This is particularly > helpful if one wants to quickly check whether a certain code has a bug > limiting > its precision away from full machine accuracy.) > > A similar promotion feature is available in Fujitsu compilers, see here: > > http://www.lahey.com/docs/fujitsu%20compiler%20option%20list.pdf > > (e.g. -CcR8R16) > > The implementation work on this was done by Zydrunas Gimbutas, not by me. > Zydrunas has authorized me to submit this for inclusion in gcc. Both he > and I have gone through the FSF's copyright assignment process and have > current papers for that on file. > > We tested the change by running Kahan's Fortran paranoia tests using all > supported conversions, we ran the LINPACK tests (at all supported > conversions) as well as a number of manually-written conversion tests. > > Zydrunas and Andreas Andreas, My apologies for letting this feature/patch fall through the cracks. I have time over the next few days to review/test/ and commit this patch. Are there any changes that I need to known about? -- Steve
Backport revision 180159 to gcc 4.6 branch
Hi, I don't really follow gcc much, so I hope I'm asking this in the right place :). I would like to ask that the fix in revision 180159 be backported to the gcc 4.6 branch. It's a trivial change, but without it clang cannot compile anything using the default std::shared_ptr constructor. That's my understanding at least, based on what I read at http://stackoverflow.com/questions/7964360/using-stdshared-ptr-with-clang-and-libstdc. Please let me know if there's any way I can help. Thanks! -- Chase
Re: [RFC]: use cgraph to emit alpha vas trampoline entry point
On Tue, Dec 20, 2011 at 9:46 AM, Tristan Gingold wrote: > Hi, > > currently alpha/vms backend emits a trampoline entry point for all nested > functions. This is a waste of code space, as although nested functions are > very common in Ada, address of nested functions are only seldom taken. > > The fact that the address of a function is taken seems only be available in > cgraph. Is it OK to use cgraph in alpha.c ? Since no-one has answered yet, I'll just toss in my $0.02. (Hold on to them a bit, they may be worth a million Euro soon :-) I think that in general you cannot rely on cgraph in the backends, this has to be analyzed case-by-case. In your case I'm not sure, but I think it should be OK. Your patch uses cgraph in alpha_start_function, which is apparently only used for the target hook ASM_DECLARE_FUNCTION_NAME. This hook is called from varasm.c:assemble_start_function(), and this is in turn only called from final.c:rest_of_handle_final() to generate assembly from RTL, and from cgraphunit.c:assemble_thunk() to output assembly for MI thunks. AFAICT cgraph should be correct and complete at the stage when those two functions are called. Therefore your patch should be OK. Perhaps Honza can throw in his 0.02h? Ciao! Steven
Re: Backport revision 180159 to gcc 4.6 branch
On 12/24/2011 10:08 AM, Chase Douglas wrote: > Hi, > > I don't really follow gcc much, so I hope I'm asking this in the right > place :). > > I would like to ask that the fix in revision 180159 be backported to the > gcc 4.6 branch. It's a trivial change, but without it clang cannot > compile anything using the default std::shared_ptr constructor. That's > my understanding at least, based on what I read at > http://stackoverflow.com/questions/7964360/using-stdshared-ptr-with-clang-and-libstdc. I realized that what I saw in viewsvn was only the tiny fix to shared_ptr. The rest of the commit implements a portion of the c++11 spec, and is not trivial. I'm attempting to build a version of 4.6 with the shared_ptr copy constructor defined as it is in the patch. When I get it working I'll send the patch here, unless there's a different procedure for stable releases? Thanks! -- Chase
