snd_usb_caiaq: use likely()/unlikely()

PATCH In review Daniel Mack 2 participants 4 messages Apr 12, 2008

Subsystems (2)

Sound Native Instruments USB sound interface driver

lore

Share & tools

lore

Daniel Mack

author

2008-04-12 20:13 UTC #0 lore raw

PATCH 1/1 snd_usb_caiaq: use likely()/unlikely()

Use likely() and unlikely() in code which is called regularily.

Signed-off-by Daniel Mack <[email protected]>

sound/usb/caiaq/caiaq-audio.c +9 -9

index 24970a5..8cb417f 100644
@@ -365,10 +365,10 @@ static void read_in_urb_mode2(struct snd_usb_caiaqdev *dev,
 
 				check_byte = MAKE_CHECKBYTE(dev, stream, i);
 				
-				if ((usb_buf[i] & 0x3f) != check_byte)
+				if (unlikely((usb_buf[i] & 0x3f) != check_byte))
 					dev->input_panic = 1;
 
-				if (usb_buf[i] & 0x80)
+				if (unlikely(usb_buf[i] & 0x80))
 					dev->output_panic = 1;
 			}
 		}

@@ -394,7 +394,7 @@ static void read_in_urb(struct snd_usb_caiaqdev *dev,
 			const struct urb *urb,
 			const struct usb_iso_packet_descriptor *iso)
 {
-	if (!dev->streaming)
+	if (unlikely(!dev->streaming))
 		return;
 
 	switch (dev->spec.data_alignment) {

@@ -406,7 +406,7 @@ static void read_in_urb(struct snd_usb_caiaqdev *dev,
 		break;
 	}
 
-	if (dev->input_panic || dev->output_panic) {
+	if (unlikely(dev->input_panic || dev->output_panic)) {
 		debug("streaming error detected %s %s\n", 
 				dev->input_panic ? "(input)" : "",
 				dev->output_panic ? "(output)" : "");

@@ -454,12 +454,12 @@ static void read_completed(struct urb *urb)
 	struct urb *out;
 	int frame, len, send_it = 0, outframe = 0;
 
-	if (urb->status || !info)
+	if (unlikely(urb->status || !info))
 		return;
 
 	dev = info->dev;
 
-	if (!dev->streaming)
+	if (unlikely(!dev->streaming))
 		return;
 
 	out = dev->data_urbs_out[info->index];

@@ -467,7 +467,7 @@ static void read_completed(struct urb *urb)
 	/* read the recently received packet and send back one which has
 	 * the same layout */
 	for (frame = 0; frame < FRAMES_PER_URB; frame++) {
-		if (urb->iso_frame_desc[frame].status)
+		if (unlikely(urb->iso_frame_desc[frame].status != 0))
 			continue;
 
 		len = urb->iso_frame_desc[outframe].actual_length;

@@ -488,7 +488,7 @@ static void read_completed(struct urb *urb)
 		outframe++;
 	}
 
-	if (send_it) {
+	if (likely(send_it)) {
 		out->number_of_packets = FRAMES_PER_URB;
 		out->transfer_flags = URB_ISO_ASAP;
 		usb_submit_urb(out, GFP_ATOMIC);

@@ -511,7 +511,7 @@ static void write_completed(struct urb *urb)
 	struct snd_usb_caiaq_cb_info *info = urb->context;
 	struct snd_usb_caiaqdev *dev = info->dev;
 
-	if (!dev->output_running) {
+	if (unlikely(!dev->output_running)) {
 		dev->output_running = 1;
 		wake_up(&dev->prepare_wait_queue);
 	}

sound/usb/caiaq/caiaq-device.c +2 -2

index e97d8b2..e1dbce5 100644
@@ -42,7 +42,7 @@
 #endif
 
 MODULE_AUTHOR("Daniel Mack <[email protected]>");
-MODULE_DESCRIPTION("caiaq USB audio, version 1.3.6");
+MODULE_DESCRIPTION("caiaq USB audio, version 1.3.7");
 MODULE_LICENSE("GPL");
 MODULE_SUPPORTED_DEVICE("{{Native Instruments, RigKontrol2},"
 			 "{Native Instruments, RigKontrol3},"

@@ -119,7 +119,7 @@ static void usb_ep1_command_reply_dispatch (struct urb* urb)
 	struct snd_usb_caiaqdev *dev = urb->context;
 	unsigned char *buf = urb->transfer_buffer;
 
-	if (urb->status || !dev) {
+	if (unlikely(urb->status || !dev)) {
 		log("received EP1 urb->status = %i\n", urb->status);
 		return;
 	}

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Adrian Bunk

2008-04-12 20:22 UTC #1 lore raw

Quoted 2 lines · Daniel Mack

Use likely() and unlikely() in code which is called regularily.
...

Does it bring any measurable advantage?

Otherwise it's usually better to let the compiler decide how to optimize the code.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Daniel Mack

author

2008-04-12 21:01 UTC #2 lore raw

Hi,

Quoted 4 lines · Adrian Bunk

Does it bring any measurable advantage?
Otherwise it's usually better to let the compiler decide how to optimize the code.

I didn't really measure, but how should the compiler know how likely a certain condition in hardware state is? I thought, in such cases it's generally a good idea to give the compiler some hints.

Daniel

Adrian Bunk

2008-04-12 21:39 UTC #3 lore raw

Quoted 11 lines · Daniel Mack

Hi,
Quoted 4 lines · Adrian Bunk
Does it bring any measurable advantage?
Otherwise it's usually better to let the compiler decide how to optimize the code.
I didn't really measure, but how should the compiler know how likely a certain condition in hardware state is? I thought, in such cases it's generally a good idea to give the compiler some hints.

gcc's heuristics often tends to guess better than programmers on how to optimize code since most people who think they know better how code should get optimized than the compiler do not actually know better...

And what you call "some hints" _forces_ the compiler to optimize for a certain condition (even if this e.g. results in bigger code or omitting some optimization).

Considering the lines of code and the number of contributors to the Linux kernel some annotations are always wrong and might cause more harm than the correct ones gain.

If it's some hotpath and a current gcc does generate measurably faster code with a likely/unlikely it's justified, but otherwise it's better to leave gcc all freedom on how to optimize it.

Quoted 1 line

Daniel

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed