| Message ID | 20170512233843.27713-2-f4bug@amsat.org |
|---|---|
| State | Changes Requested |
| Headers | show |
On 05/12/2017 06:38 PM, Philippe Mathieu-Daudé wrote: > If you have coccinelle installed you can apply this script using: > > $ spatch \ > --macro-file scripts/cocci-macro-file.h \ > --dir target --in-place > > You can also use directly Peter Senna Tschudin docker image (easier): > > $ docker run -v `pwd`:`pwd` -w `pwd` petersenna/coccinelle \ > --sp-file scripts/coccinelle/tcg_gen_extract.cocci \ > --macro-file scripts/cocci-macro-file.h \ > --dir target --in-place > > Then verified that no manual touchups are required. > > The following thread was helpful while writing this script: > > https://github.com/coccinelle/coccinelle/issues/86 > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > scripts/coccinelle/tcg_gen_extract.cocci | 103 +++++++++++++++++++++++++++++++ > 1 file changed, 103 insertions(+) > create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci It's still not obvious to me whether we want this script in the tree (as something we plan to rerun regularly to check for regressions), or just in the commit message (useful for the one-time location of spots to optimize, but something we don't anticipate repeating). > +@@ > +import sys > +fd = sys.stderr > +def debug(msg="", trailer="\n"): > + fd.write("[DBG] " + msg + trailer) > +def low_bits_count(value): > + bits_count = 0 > + while (value & (1 << bits_count)): > + bits_count += 1 Surely python has a faster method than this (after all, we have ctz and friends in C code)? But my python is limited enough that I don't know of one off-hand. > + return bits_count > +def Mn(order): # Mersenne number > + return (1 << order) - 1 Correct name... > +else: > + debug("candidate at %s:%s" % (shr_p[0].file, shr_p[0].line)) > + try: # only eval integer, no #define like 'SR_M' (cpp did this, else some headers are missing). > + msk_v = long(msk_s.strip("UL"), 0) > + msk_b = low_bits_count(msk_v) > + if msk_b == 0: > + debug(" value: 0x%x low_bits: %d" % (msk_v, msk_b)) > + else: > + debug(" value: 0x%x low_bits: %d [Mersenne prime: 0x%x]" % (msk_v, msk_b, Mn(msk_b))) ...but this name is still wrong.
On 15/05/2017 16:04, Eric Blake wrote: > On 05/12/2017 06:38 PM, Philippe Mathieu-Daudé wrote: >> If you have coccinelle installed you can apply this script using: >> >> $ spatch \ >> --macro-file scripts/cocci-macro-file.h \ >> --dir target --in-place >> >> You can also use directly Peter Senna Tschudin docker image (easier): >> >> $ docker run -v `pwd`:`pwd` -w `pwd` petersenna/coccinelle \ >> --sp-file scripts/coccinelle/tcg_gen_extract.cocci \ >> --macro-file scripts/cocci-macro-file.h \ >> --dir target --in-place >> >> Then verified that no manual touchups are required. >> >> The following thread was helpful while writing this script: >> >> https://github.com/coccinelle/coccinelle/issues/86 >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> scripts/coccinelle/tcg_gen_extract.cocci | 103 +++++++++++++++++++++++++++++++ >> 1 file changed, 103 insertions(+) >> create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci > > It's still not obvious to me whether we want this script in the tree (as > something we plan to rerun regularly to check for regressions), or just > in the commit message (useful for the one-time location of spots to > optimize, but something we don't anticipate repeating). I think it's useful. New backends can have this issue, plus it shows some advanced Coccinelle techniques. Paolo > > >> +@@ >> +import sys >> +fd = sys.stderr >> +def debug(msg="", trailer="\n"): >> + fd.write("[DBG] " + msg + trailer) >> +def low_bits_count(value): >> + bits_count = 0 >> + while (value & (1 << bits_count)): >> + bits_count += 1 > > Surely python has a faster method than this (after all, we have ctz and > friends in C code)? But my python is limited enough that I don't know > of one off-hand. > >> + return bits_count >> +def Mn(order): # Mersenne number >> + return (1 << order) - 1 > > Correct name... > > >> +else: >> + debug("candidate at %s:%s" % (shr_p[0].file, shr_p[0].line)) >> + try: # only eval integer, no #define like 'SR_M' (cpp did this, else some headers are missing). >> + msk_v = long(msk_s.strip("UL"), 0) >> + msk_b = low_bits_count(msk_v) >> + if msk_b == 0: >> + debug(" value: 0x%x low_bits: %d" % (msk_v, msk_b)) >> + else: >> + debug(" value: 0x%x low_bits: %d [Mersenne prime: 0x%x]" % (msk_v, msk_b, Mn(msk_b))) > > ...but this name is still wrong. >
On 15/05/2017 16:06, Paolo Bonzini wrote: > > > On 15/05/2017 16:04, Eric Blake wrote: >> On 05/12/2017 06:38 PM, Philippe Mathieu-Daudé wrote: >>> If you have coccinelle installed you can apply this script using: >>> >>> $ spatch \ >>> --macro-file scripts/cocci-macro-file.h \ >>> --dir target --in-place >>> >>> You can also use directly Peter Senna Tschudin docker image (easier): >>> >>> $ docker run -v `pwd`:`pwd` -w `pwd` petersenna/coccinelle \ >>> --sp-file scripts/coccinelle/tcg_gen_extract.cocci \ >>> --macro-file scripts/cocci-macro-file.h \ >>> --dir target --in-place >>> >>> Then verified that no manual touchups are required. >>> >>> The following thread was helpful while writing this script: >>> >>> https://github.com/coccinelle/coccinelle/issues/86 >>> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> --- >>> scripts/coccinelle/tcg_gen_extract.cocci | 103 +++++++++++++++++++++++++++++++ >>> 1 file changed, 103 insertions(+) >>> create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci >> >> It's still not obvious to me whether we want this script in the tree (as >> something we plan to rerun regularly to check for regressions), or just >> in the commit message (useful for the one-time location of spots to >> optimize, but something we don't anticipate repeating). > > I think it's useful. New backends can have this issue, plus it shows > some advanced Coccinelle techniques. > I agree: I think it's a good idea to have a place in QEMU directory with all past coccinelle scripts to help to write the new ones. Laurent
diff --git a/scripts/coccinelle/tcg_gen_extract.cocci b/scripts/coccinelle/tcg_gen_extract.cocci new file mode 100644 index 0000000000..37546834ee --- /dev/null +++ b/scripts/coccinelle/tcg_gen_extract.cocci @@ -0,0 +1,103 @@ +// optimize TCG using extract op +// +// Copyright: (C) 2017 Philippe Mathieu-Daudé. GPLv2+. +// Confidence: High +// Options: --macro-file scripts/cocci-macro-file.h +// +// Nikunj A Dadhania optimization: +// http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05211.html +// Aurelien Jarno optimization: +// http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg01466.html +// Coccinelle helpful issue: +// https://github.com/coccinelle/coccinelle/issues/86 + +@initialize:python@ +@@ +import sys +fd = sys.stderr +def debug(msg="", trailer="\n"): + fd.write("[DBG] " + msg + trailer) +def low_bits_count(value): + bits_count = 0 + while (value & (1 << bits_count)): + bits_count += 1 + return bits_count +def Mn(order): # Mersenne number + return (1 << order) - 1 + +@match@ // depends on never match_and_check_reg_used@ +metavariable ret, arg; +constant ofs, msk; +expression tcg_arg; +identifier tcg_func =~ "^tcg_gen_"; +position shr_p, and_p; +@@ +( + tcg_gen_shri_i32@shr_p +| + tcg_gen_shri_i64@shr_p +| + tcg_gen_shri_tl@shr_p +)(ret, arg, ofs); +<... +tcg_func(tcg_arg, ...); +...> +( + tcg_gen_andi_i32@and_p +| + tcg_gen_andi_i64@and_p +| + tcg_gen_andi_tl@and_p +)(ret, ret, msk); + +@script:python verify_len depends on match@ +ret_s << match.ret; +msk_s << match.msk; +shr_p << match.shr_p; +tcg_func << match.tcg_func; +tcg_arg << match.tcg_arg; +extract_len; +@@ +is_optimizable = False +debug("candidate at %s:%s" % (shr_p[0].file, shr_p[0].line)) +if tcg_arg == ret_s: + debug(" %s() modifies argument '%s'" % (tcg_func, ret_s)) +else: + debug("candidate at %s:%s" % (shr_p[0].file, shr_p[0].line)) + try: # only eval integer, no #define like 'SR_M' (cpp did this, else some headers are missing). + msk_v = long(msk_s.strip("UL"), 0) + msk_b = low_bits_count(msk_v) + if msk_b == 0: + debug(" value: 0x%x low_bits: %d" % (msk_v, msk_b)) + else: + debug(" value: 0x%x low_bits: %d [Mersenne prime: 0x%x]" % (msk_v, msk_b, Mn(msk_b))) + is_optimizable = Mn(msk_b) == msk_v # check low_bits + coccinelle.extract_len = "%d" % msk_b + debug(" candidate %s optimizable" % ("IS" if is_optimizable else "is NOT")) + except: + debug(" ERROR (check included headers?)") +cocci.include_match(is_optimizable) +debug() + +@replacement depends on verify_len@ +metavariable match.ret, match.arg; +constant match.ofs, match.msk; +position match.shr_p, match.and_p; +identifier verify_len.extract_len; +@@ +( +-tcg_gen_shri_i32@shr_p(ret, arg, ofs); ++tcg_gen_extract_i32(ret, arg, ofs, extract_len); +... +-tcg_gen_andi_i32@and_p(ret, ret, msk); +| +-tcg_gen_shri_i64@shr_p(ret, arg, ofs); ++tcg_gen_extract_i64(ret, arg, ofs, extract_len); +... +-tcg_gen_andi_i64@and_p(ret, ret, msk); +| +-tcg_gen_shri_tl@shr_p(ret, arg, ofs); ++tcg_gen_extract_tl(ret, arg, ofs, extract_len); +... +-tcg_gen_andi_tl@and_p(ret, ret, msk); +)
If you have coccinelle installed you can apply this script using: $ spatch \ --macro-file scripts/cocci-macro-file.h \ --dir target --in-place You can also use directly Peter Senna Tschudin docker image (easier): $ docker run -v `pwd`:`pwd` -w `pwd` petersenna/coccinelle \ --sp-file scripts/coccinelle/tcg_gen_extract.cocci \ --macro-file scripts/cocci-macro-file.h \ --dir target --in-place Then verified that no manual touchups are required. The following thread was helpful while writing this script: https://github.com/coccinelle/coccinelle/issues/86 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- scripts/coccinelle/tcg_gen_extract.cocci | 103 +++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci